Thread: Initial review of xslt with no limits patch
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? 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, 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
Hello 2010/8/2 Mike Fowler <mike@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
Attachment
Hi Pavel,
On 02/08/10 09:21, Pavel Stehule wrote:
> Hello
>
> 2010/8/2 Mike Fowler<mike@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:paramname="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-ofselect="$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:elementname="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
2010/8/6 Mike Fowler <mike@mlfowler.com>:
> Hi Pavel,
>
> On 02/08/10 09:21, Pavel Stehule wrote:
>>
>> Hello
>>
>> 2010/8/2 Mike Fowler<mike@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
>
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
2010/8/6 Andrew Dunstan <andrew@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
>
On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
> 2010/8/6 Andrew Dunstan <andrew@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'
This sounds like the perfect case for pulling hstore into core code. :)
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
2010/8/6 David Fetter <david@fetter.org>:
> On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
>> 2010/8/6 Andrew Dunstan <andrew@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'
>
> This sounds like the perfect case for pulling hstore into core code. :)
I afraid so integration of hstore can break and block work on real
hash support. I would to have hash tables in core, but with usual
features and usual syntax - like Perl or PHP
Regards
Pavel
>
> Cheers,
> David.
> --
> David Fetter <david@fetter.org> http://fetter.org/
> Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
> Skype: davidfetter XMPP: david.fetter@gmail.com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate
>
On 08/06/2010 02:29 AM, Pavel Stehule wrote:
> 2010/8/6 David Fetter<david@fetter.org>:
>> On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
>>> 2010/8/6 Andrew Dunstan<andrew@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'
>> This sounds like the perfect case for pulling hstore into core code. :)
> I afraid so integration of hstore can break and block work on real
> hash support. I would to have hash tables in core, but with usual
> features and usual syntax - like Perl or PHP
>
Can we just keep this discussion within reasonable bounds? The issue is
not hstore or other hashes, but how to do the param list for xslt sanely
given what we have now. A variadic list will be much nicer than what is
currently proposed.
cheers
andrew
On 06/08/10 15:08, Andrew Dunstan wrote:
>
>
> On 08/06/2010 02:29 AM, Pavel Stehule wrote:
>> 2010/8/6 David Fetter<david@fetter.org>:
>>> On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
>>>> 2010/8/6 Andrew Dunstan<andrew@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'
>>> This sounds like the perfect case for pulling hstore into core code. :)
>> I afraid so integration of hstore can break and block work on real
>> hash support. I would to have hash tables in core, but with usual
>> features and usual syntax - like Perl or PHP
>>
>
> Can we just keep this discussion within reasonable bounds? The issue
> is not hstore or other hashes, but how to do the param list for xslt
> sanely given what we have now. A variadic list will be much nicer than
> what is currently proposed.
>
> cheers
>
> andrew
+1
Variadic seems the most sensible to me anyways.
However the more urgent problem is, pending someone spotting an error in
my ways, neither the existing code or the patched code appear able to
evaluate the parameters. Note than in my example I supplied a default
value for the fifth parameter and not even that value is appearing in
the outputted XML.
Regards,
--
Mike Fowler
Registered Linux user: 379787
Mike Fowler <mike@mlfowler.com> writes: > SELECT > xslt_process( ... , ... , > 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) produces > <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> Some examination of http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html suggests that the parameter values need to be single-quoted, and indeed when I change the last part of your example to 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); I get xslt_process -----------------------<samples> + <sample>v1</sample>+ <sample>v2</sample>+ <sample>v3</sample>+ <sample>v4</sample>+ <sample>v5</sample>+</samples> + (1 row) So this seems to be a documentation problem more than a code problem. (It's a bit distressing to notice that the regression tests for the module fail to exercise 3-parameter xslt_process at all, though.) regards, tom lane
Hello attached updated patch with regression test 2010/8/6 Tom Lane <tgl@sss.pgh.pa.us>: > Mike Fowler <mike@mlfowler.com> writes: >> SELECT >> xslt_process( ... , ... , >> 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) > > produces > >> <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> > > Some examination of > http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html > suggests that the parameter values need to be single-quoted, > and indeed when I change the last part of your example to > > 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); > > I get > > xslt_process > ----------------------- > <samples> + > <sample>v1</sample>+ > <sample>v2</sample>+ > <sample>v3</sample>+ > <sample>v4</sample>+ > <sample>v5</sample>+ > </samples> + > > (1 row) > > So this seems to be a documentation problem more than a code problem. > > (It's a bit distressing to notice that the regression tests for the > module fail to exercise 3-parameter xslt_process at all, though.) > ??? I don't see it Regards Pavel Stehule > regards, tom lane >
Attachment
On 08/06/2010 12:15 PM, Tom Lane wrote: > > Some examination of > http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html > suggests that the parameter values need to be single-quoted, > and indeed when I change the last part of your example to > > 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); > > Which would look a whole lot nicer with dollar quoting ;-) cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 08/06/2010 12:15 PM, Tom Lane wrote: >> Some examination of >> http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html >> suggests that the parameter values need to be single-quoted, >> and indeed when I change the last part of your example to >> >> 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); > Which would look a whole lot nicer with dollar quoting ;-) No doubt. But one would assume that constant parameters aren't going to be the normal use-case, and dollar quoting isn't helpful for nonconstant text. I think there are issues here that we need to take a step back and think about. Right now, thanks to the lack of documentation, we can probably assume there are approximately zero users of the xslt_process parameter feature. Once we document it that'll no longer be true. So right now would be the time to reflect on whether this is a specification we actually like or believe is usable; it'll be too late to change it later. There are two specific points bothering me now that I see how it works: 1. name = value pretty much sucks, especially with the 100% lack of any quoting convention for either equals or comma. I concur with the thoughts upthread that turning this into a variadic function would be a more sensible solution. 2. I'm not sure whether we ought to auto-single-quote the values. If we don't, how hard is it for users to properly quote nonconstant parameter values? (Will quote_literal work, or are the quoting rules different for libxslt?) If we do, are we giving up functionality someone cares about? regards, tom lane
2010/8/6 Tom Lane <tgl@sss.pgh.pa.us>: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 08/06/2010 12:15 PM, Tom Lane wrote: >>> Some examination of >>> http://www.xmlsoft.org/XSLT/tutorial/libxslttutorial.html >>> suggests that the parameter values need to be single-quoted, >>> and indeed when I change the last part of your example to >>> >>> 'n1=''v1'',n2=''v2'',n3=''v3'',n4=''v4'',n5=''v5'''::text); > >> Which would look a whole lot nicer with dollar quoting ;-) > > No doubt. But one would assume that constant parameters aren't going > to be the normal use-case, and dollar quoting isn't helpful for > nonconstant text. > > I think there are issues here that we need to take a step back and think > about. Right now, thanks to the lack of documentation, we can probably > assume there are approximately zero users of the xslt_process parameter > feature. Once we document it that'll no longer be true. So right now > would be the time to reflect on whether this is a specification we > actually like or believe is usable; it'll be too late to change it > later. > I know about one important user from Czech Republic > There are two specific points bothering me now that I see how it works: > > 1. name = value pretty much sucks, especially with the 100% lack of any > quoting convention for either equals or comma. I concur with the > thoughts upthread that turning this into a variadic function would be a > more sensible solution. I'll propose a new kind of functions (only position parameter's function). My idea is simple - for functions with this mark the mixed and named notation is blocked. But these functions can have a parameter names - and these names can be passed to function. So there is possible have a xslt_process function with current behave - third argument has not label, and new variadic version like xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2', param_name3 = 'v3', ...) an implementation of this functionality can be very simple, and we can use this for xslt_process in 9.1 Regards Pavel Stehule > > 2. I'm not sure whether we ought to auto-single-quote the values. > If we don't, how hard is it for users to properly quote nonconstant > parameter values? (Will quote_literal work, or are the quoting rules > different for libxslt?) If we do, are we giving up functionality > someone cares about? > > regards, tom lane >
On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > I'll propose a new kind of functions (only position parameter's > function). My idea is simple - for functions with this mark the mixed > and named notation is blocked. But these functions can have a > parameter names - and these names can be passed to function. So there > is possible have a > > xslt_process function with current behave - third argument has not > label, and new variadic version like > > xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2', > param_name3 = 'v3', ...) > > an implementation of this functionality can be very simple, and we can > use this for xslt_process in 9.1 Why wouldn't we just pass two text arrays to this function and be done with it? Custom syntax is all well and good when you're writing these calls by hand, but it's not hard to imagine someone wanting to pass in values stored somewhere else. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2010/8/6 Tom Lane <tgl@sss.pgh.pa.us>:
>> I think there are issues here that we need to take a step back and think
>> about. Right now, thanks to the lack of documentation, we can probably
>> assume there are approximately zero users of the xslt_process parameter
>> feature. Once we document it that'll no longer be true. So right now
>> would be the time to reflect on whether this is a specification we
>> actually like or believe is usable; it'll be too late to change it
>> later.
> I know about one important user from Czech Republic
Well, if there actually is anybody who's figured it out, we could easily
have a backwards-compatible mode. Provide one variadic function that
acts as follows:even number of variadic array elements -> they're names/valuesone variadic array element -> parse it
theold wayotherwise -> error
I wouldn't even bother with fixing the MAXPARAMS limitation for the
"old way" code, just make it work exactly as before.
> I'll propose a new kind of functions (only position parameter's
> function). My idea is simple - for functions with this mark the mixed
> and named notation is blocked.
We don't need random new function behaviors for this. Anyway your
proposal doesn't work at all for non-constant parameter names.
regards, tom lane
2010/8/6 Robert Haas <robertmhaas@gmail.com>:
> On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> I'll propose a new kind of functions (only position parameter's
>> function). My idea is simple - for functions with this mark the mixed
>> and named notation is blocked. But these functions can have a
>> parameter names - and these names can be passed to function. So there
>> is possible have a
>>
>> xslt_process function with current behave - third argument has not
>> label, and new variadic version like
>>
>> xslt_process(..,.., param_name1 = 'v1', param_name2 = 'v2',
>> param_name3 = 'v3', ...)
>>
>> an implementation of this functionality can be very simple, and we can
>> use this for xslt_process in 9.1
>
> Why wouldn't we just pass two text arrays to this function and be done
> with it? Custom syntax is all well and good when you're writing these
> calls by hand, but it's not hard to imagine someone wanting to pass in
> values stored somewhere else.
yes, it is possible too. And maybe is better then current
xslt_process. But it isn't too much readable and robust. You have to
calculate position of name and position of value. This is same in
other languages. You can use a dynamic parameters in PHP or Perl via
two arrays, but nobody use it. People use a hash syntax (param1=>val,
param2=>val). This proposal isn't only for xslt_process function. Why
hstore has a custom parser? It can use only a pair of arrays too.
For Tom: proposed syntax can be used generally - everywhere when you
are working with collection. It can be used for hash (hstore)
constructor for example. For me is more readable code like
select hstore(name := 'Tomas', surname := 'Novak')
than
select hstore('name=\'Tomas\', surname=\'Novak\'')
The main advance of this feature is simplicity of custom functions.
Its must not have a custom parser. So possible an using is hstore,
xslt_process
Regards
Pavel Stehule
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise Postgres Company
>
Robert Haas <robertmhaas@gmail.com> writes:
> Why wouldn't we just pass two text arrays to this function and be done
> with it?
That would work too, although I think it might be a bit harder to use
than one alternating-name-and-value array, at least in some scenarios.
You'd have to be careful that you got the values in the same order in
both arrays, which'd be easy to botch.
There might be other use-cases where two separate arrays are easier
to use, but I'm not seeing one offhand.
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
> For Tom: proposed syntax can be used generally - everywhere when you
> are working with collection. It can be used for hash (hstore)
> constructor for example. For me is more readable code like
> select hstore(name := 'Tomas', surname := 'Novak')
You've tried to sell us on that before, with few takers. This proposed
use-case impresses me even less than the previous ones, because callers
of xslt_process seem quite likely to need to work with non-constant
parameter names.
In any case, given what we have at the moment for function overload
resolution rules, I think it's a fundamentally bad idea to introduce
a "wild card" function type that would necessarily conflict with
practically every other possible function declaration. So regardless
of what use-cases you propose, I'm going to vote against that.
regards, tom lane
2010/8/6 Tom Lane <tgl@sss.pgh.pa.us>:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
>> For Tom: proposed syntax can be used generally - everywhere when you
>> are working with collection. It can be used for hash (hstore)
>> constructor for example. For me is more readable code like
>
>> select hstore(name := 'Tomas', surname := 'Novak')
>
> You've tried to sell us on that before, with few takers. This proposed
> use-case impresses me even less than the previous ones, because callers
> of xslt_process seem quite likely to need to work with non-constant
> parameter names.
>
> In any case, given what we have at the moment for function overload
> resolution rules, I think it's a fundamentally bad idea to introduce
> a "wild card" function type that would necessarily conflict with
> practically every other possible function declaration. So regardless
> of what use-cases you propose, I'm going to vote against that.
It must not be a function. Just I missing any tool that helps with
complex structured data. This proposed kind functions has one
advantage - there isn't necessary any change in parser. Yes, I can use
a pair of arrays, I can use a one array with seq name, value, I can
use a custom parser. But nothing from these offers a comfort or
readability for example a Perl's hash tables.
so if we have a integrated hash tables, then we can to write some like
CREATE FUNCTION xslt_process(..,.., text{})
select xslt_process(..,.., { par1 => val1, par2 => val3, .. } )
any simple method can significantly help for us, who write a lot of
complex stored procedures. It can be a big help. I am only
dissatisfied because it is "Perlism" - maybe I don't understand SQL
well, but my personal opinion about the most natural syntax for this
situation is some like SQL/XML - xmlattributes or named notation. SQL
isn't too much consistent too - it uses two semantically similar
syntax
foo(name=>value, ..) versus foo(value AS name, ..)
Next my idea was mix of named parameters and marked variadic parameter:
CREATE FUNCTION xslt_process(..,.., VARIADIC LABELED text[])
and then we can call SELECT xslt_process(..,.., par1 := var, ..)
This doesn't introduce a heavy new syntax - just use old in some
specific context. It is only my feeling, so this way is more SQL
natural than using a some hash data type. Maybe you don't think, so
stored procedures must not to have a this functionality. Everywhere
where you can to wrap a c library for SQL you have to meet this task -
often you have to pass a set of flags, collection of values. With
default parameter values situation is better then before, but its
still not ideal - for example SOAP interface or dblink to Oracle
SELECT exec(db, 'SELECT * FROM foo where c = :name', name => 'Pavel')
so this is my motivation - the possibility to write generic custom
functions. Sure - this is "redundant" to existing functionality. I can
write:
SELECT exec(db, 'SELECT * FROM ...', ARRAY['name'], ARRAY['Pavel']) --
Robert'syntax
or your syntax:
SELECT exec(db, 'SELECT * FROM ...', ARRAY['name','Pavel']) or
like current xml2 syntax:
SELECT exec(db, 'SELECT * FROM ...', 'name="Pavel")
But these versions are "too simple" if you understand me. It doesn't
do life of SQL stored procedure's programmer simper.
Regards
Pavel
p.s. sorry for offtopic
p.s. for me isn't important notation. Just I would to like more things
with custom functions withou parser modification.
>
> regards, tom lane
>
On Aug 6, 2010, at 11:13 AM, Tom Lane wrote: > That would work too, although I think it might be a bit harder to use > than one alternating-name-and-value array, at least in some scenarios. > You'd have to be careful that you got the values in the same order in > both arrays, which'd be easy to botch. > > There might be other use-cases where two separate arrays are easier > to use, but I'm not seeing one offhand. Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then you'd just have a function with `variadic orderedpair` as the signature. I don't suppose anyone has implemented a data type like this… Best, David
On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote: > 2. I'm not sure whether we ought to auto-single-quote the values. > If we don't, how hard is it for users to properly quote nonconstant > parameter values? (Will quote_literal work, or are the quoting rules > different for libxslt?) If we do, are we giving up functionality > someone cares about? Not every parameter is a string. Compare xsltproc: --param PARAMNAME PARAMVALUE Pass a parameter of name PARAMNAME and value PARAMVALUE to the stylesheet. You maypass multiple name/value pairs up to a maximum of 32. If the value being passed is a string, you can use --stringparaminstead, to avoid additional quote characters that appear in string expressions. Note: the XPath expression must be UTF-8 encoded. --stringparam PARAMNAME PARAMVALUE Pass a parameter of name PARAMNAME and value PARAMVALUE where PARAMVALUE isa string rather than a node identifier. Note: The string must be UTF-8 encoded.
On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: > It must not be a function. Just I missing any tool that helps with > complex structured data. This proposed kind functions has one > advantage - there isn't necessary any change in parser. Yes, I can use > a pair of arrays, I can use a one array with seq name, value, I can > use a custom parser. But nothing from these offers a comfort or > readability for example a Perl's hash tables. Maybe you should just use PL/XSLT. :-)
2010/8/6 David E. Wheeler <david@kineticode.com>: > On Aug 6, 2010, at 11:13 AM, Tom Lane wrote: > >> That would work too, although I think it might be a bit harder to use >> than one alternating-name-and-value array, at least in some scenarios. >> You'd have to be careful that you got the values in the same order in >> both arrays, which'd be easy to botch. >> >> There might be other use-cases where two separate arrays are easier >> to use, but I'm not seeing one offhand. > > Stuff like this makes me wish PostgreSQL had an ordered pair data type. Then you'd just have a function with `variadicordered pair` as the signature. > yes it is one a possibility and probably best. The nice of this variant can be two forms like current variadic does - foo(.., a := 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)]) > I don't suppose anyone has implemented a data type like this… > > Best, > > David > >
2010/8/6 Peter Eisentraut <peter_e@gmx.net>: > On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: >> It must not be a function. Just I missing any tool that helps with >> complex structured data. This proposed kind functions has one >> advantage - there isn't necessary any change in parser. Yes, I can use >> a pair of arrays, I can use a one array with seq name, value, I can >> use a custom parser. But nothing from these offers a comfort or >> readability for example a Perl's hash tables. > > Maybe you should just use PL/XSLT. :-) > :)
> yes it is one a possibility and probably best. The nice of this
> variant can be two forms like current variadic does - foo(.., a :=
> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])
>
>
pardon foo(..., VARIADIC ARRAY[('a', 10), ('b' 10)])
regards
Pavel
On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote:
> yes it is one a possibility and probably best. The nice of this
> variant can be two forms like current variadic does - foo(.., a :=
> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])
I started fiddling and got as far as this:
CREATE TYPE pair AS ( key text, val text );
CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair
LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair;
$$;
CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair
LANGUAGE SQL AS $$ SELECT ROW($1, $2)::pair;
$$;
CREATE OPERATOR ~> (LEFTARG = anyelement,RIGHTARG = anyelement,PROCEDURE = pair
);
CREATE OPERATOR ~> (LEFTARG = text,RIGHTARG = text,PROCEDURE = pair
);
CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text
LANGUAGE SQL AS $$
-- SELECT unnest($1)::text SELECT $1[1].key UNION SELECT $1[1].val UNION SELECT $1[2].key UNION SELECT
$1[2].val;
$$;
SELECT foo('this' ~> 'that', 1 ~> 4);
Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the new
namefor =>).
Best,
David
2010/8/6 David E. Wheeler <david@kineticode.com>:
> On Aug 6, 2010, at 1:49 PM, Pavel Stehule wrote:
>
>> yes it is one a possibility and probably best. The nice of this
>> variant can be two forms like current variadic does - foo(.., a :=
>> 10, b := 10) or foo(.., variadic ARRAY[(a,10),(b,10)])
>
> I started fiddling and got as far as this:
>
>
> CREATE TYPE pair AS ( key text, val text );
>
> CREATE OR REPLACE FUNCTION pair(anyelement, anyelement) RETURNS pair
> LANGUAGE SQL AS $$
> SELECT ROW($1, $2)::pair;
> $$;
>
> CREATE OR REPLACE FUNCTION pair(text, text) RETURNS pair
> LANGUAGE SQL AS $$
> SELECT ROW($1, $2)::pair;
> $$;
>
> CREATE OPERATOR ~> (
> LEFTARG = anyelement,
> RIGHTARG = anyelement,
> PROCEDURE = pair
> );
>
> CREATE OPERATOR ~> (
> LEFTARG = text,
> RIGHTARG = text,
> PROCEDURE = pair
> );
>
> CREATE OR REPLACE FUNCTION foo(variadic pair[]) RETURNS SETOF text
> LANGUAGE SQL AS $$
> -- SELECT unnest($1)::text
> SELECT $1[1].key
> UNION SELECT $1[1].val
> UNION SELECT $1[2].key
> UNION SELECT $1[2].val;
> $$;
>
> SELECT foo('this' ~> 'that', 1 ~> 4);
>
> Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the
newname for =>).
so there is only small step to proposed feature
SELECT foo(this := 'that', "1" := 4)
there is only one difference (but you cannot implement it now)
* notation for key is same like for sql identifier - why: I would to
clearly identify key and value. When I use a custom operator - like
you did, it depends on implementation what is key, what is value. When
you use a SQL identifier's notation for key, you can't to do a error
Regards
Pavel
>
> Best,
>
> David
>
>
>
On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote:
>> SELECT foo('this' ~> 'that', 1 ~> 4);
>>
>> Not bad, I think. I kind of like it. It reminds me how much I hate the % hstore construction operator, though (the
newname for =>).
>
> so there is only small step to proposed feature
>
> SELECT foo(this := 'that', "1" := 4)
>
> there is only one difference (but you cannot implement it now)
> * notation for key is same like for sql identifier - why: I would to
> clearly identify key and value. When I use a custom operator - like
> you did, it depends on implementation what is key, what is value. When
> you use a SQL identifier's notation for key, you can't to do a error
Sorry, not following you here…
David
Peter Eisentraut <peter_e@gmx.net> writes:
> On fre, 2010-08-06 at 13:01 -0400, Tom Lane wrote:
>> 2. I'm not sure whether we ought to auto-single-quote the values.
>> If we don't, how hard is it for users to properly quote nonconstant
>> parameter values? (Will quote_literal work, or are the quoting rules
>> different for libxslt?) If we do, are we giving up functionality
>> someone cares about?
> Not every parameter is a string.
So I gather, but what else is there, and do we actually want to expose
the other alternatives in xslt_process()?
If we don't auto-quote, we need to provide some sort of quote_xslt()
function that will apply the appropriate quoting/escaping to deal
with an arbitrary string value.
regards, tom lane
"David E. Wheeler" <david@kineticode.com> writes:
> On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote:
>> so there is only small step to proposed feature
>> SELECT foo(this := 'that', "1" := 4)
> Sorry, not following you here
Pavel doesn't understand "no" ;-)
regards, tom lane
On Fri, Aug 06, 2010 at 11:48:58PM +0300, Peter Eisentraut wrote: > On fre, 2010-08-06 at 21:31 +0200, Pavel Stehule wrote: > > It must not be a function. Just I missing any tool that helps with > > complex structured data. This proposed kind functions has one > > advantage - there isn't necessary any change in parser. Yes, I can > > use a pair of arrays, I can use a one array with seq name, value, > > I can use a custom parser. But nothing from these offers a comfort > > or readability for example a Perl's hash tables. > > Maybe you should just use PL/XSLT. :-) When's that going into the tree? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
2010/8/7 Tom Lane <tgl@sss.pgh.pa.us>: > "David E. Wheeler" <david@kineticode.com> writes: >> On Aug 6, 2010, at 2:12 PM, Pavel Stehule wrote: >>> so there is only small step to proposed feature >>> SELECT foo(this := 'that', "1" := 4) > >> Sorry, not following you here I would to difference a key and value in notation. > > Pavel doesn't understand "no" ;-) > you are don't writing a stored procedures like me - so maybe you are doesn't understand a my motivation. :). I have to try it. You are rejected almost of all my proposals - named parameters, variadic functions, enhancing of RAISE STATEMENT - and now its in core. But it was a battle :). Try to write a XML-RPC support for PostgreSQL, and try to thinking on programmer comfort, please. I am sure so our support for stored procedures or external procedures are not complete - it is limited by BISON possibilities, and because BISON isn't extensible parser, I am searching other ways. If I can enhance a syntax from external module, I don't talk. Regards Pavel Regards Pavel > regards, tom lane >
On Aug 6, 2010, at 8:49 PM, Pavel Stehule wrote: >> Sorry, not following you here > > I would to difference a key and value in notation. That's exactly what my solution does. The array solution doesn't. Whether it's appropriate to use a custom composite type,however, is an open question. >> Pavel doesn't understand "no" ;-) > > you are don't writing a stored procedures like me - so maybe you are > doesn't understand a my motivation. :). I have to try it. You are > rejected almost of all my proposals - named parameters, variadic > functions, enhancing of RAISE STATEMENT - and now its in core. But it > was a battle :). This is how most stuff gets in: you fight Tom to exhaustion. It's a slog, but usually the resulting implementation is betterthan it would otherwise have been. > Try to write a XML-RPC support for PostgreSQL, and > try to thinking on programmer comfort, please. I am sure so our > support for stored procedures or external procedures are not complete > - it is limited by BISON possibilities, and because BISON isn't > extensible parser, I am searching other ways. If I can enhance a > syntax from external module, I don't talk. I think that some sort of variadic pairs would be useful for this. But since there is no core "ordered pair" data type, Idon't think you're going to get too far. Best, David
2010/8/7 David E. Wheeler <david@kineticode.com>: > On Aug 6, 2010, at 8:49 PM, Pavel Stehule wrote: > >>> Sorry, not following you here >> >> I would to difference a key and value in notation. > > That's exactly what my solution does. The array solution doesn't. Whether it's appropriate to use a custom composite type,however, is an open question. no it doesn't - in your design there are no different notation for key and for value. Next this design block a '->'. Because it's based on polymorphic operator. But it can be a one variant - where you would to put together expr with expr. And you can't do more from user space now. But if you have a build in operator for (sqlidentifier, any) with early processing - like "AS" in xml_attributies, we can do it. The using of this operator can be limited only on function parameter context. > >>> Pavel doesn't understand "no" ;-) >> >> you are don't writing a stored procedures like me - so maybe you are >> doesn't understand a my motivation. :). I have to try it. You are >> rejected almost of all my proposals - named parameters, variadic >> functions, enhancing of RAISE STATEMENT - and now its in core. But it >> was a battle :). > > This is how most stuff gets in: you fight Tom to exhaustion. It's a slog, but usually the resulting implementation is betterthan it would otherwise have been. > >> Try to write a XML-RPC support for PostgreSQL, and >> try to thinking on programmer comfort, please. I am sure so our >> support for stored procedures or external procedures are not complete >> - it is limited by BISON possibilities, and because BISON isn't >> extensible parser, I am searching other ways. If I can enhance a >> syntax from external module, I don't talk. > > I think that some sort of variadic pairs would be useful for this. But since there is no core "ordered pair" data type,I don't think you're going to get too far. Postgres has a array of rows (Inside C or plperlu can be transofmed to real hash simply). It just miss a user friendly notation for using it. Regards Pavel > > Best, > > David > >
"David E. Wheeler" <david@kineticode.com> writes:
> I think that some sort of variadic pairs would be useful for this. But since there is no core "ordered pair" data
type,I don't think you're going to get too far.
It's not immediately clear to me what an ordered-pair type would get you
that you don't get with 2-element arrays.
A couple of quick experiments suggest that 2-D arrays might be the thing
to use. They're easy to construct:
regression=# select array[[1,2],[3,4]]; array
---------------{{1,2},{3,4}}
(1 row)
and you can build them dynamically at need:
regression=# select array[[1,2],[3,4]] || array[5,6]; ?column?
---------------------{{1,2},{3,4},{5,6}}
(1 row)
This is not exactly without precedent, either: our built-in xpath()
function appears to use precisely this approach for its namespace-list
argument.
regards, tom lane
On Aug 6, 2010, at 9:48 PM, Pavel Stehule wrote: >> That's exactly what my solution does. The array solution doesn't. Whether it's appropriate to use a custom composite type,however, is an open question. > > no it doesn't - in your design there are no different notation for key > and for value. Next this design block a '->'. Because it's based on > polymorphic operator. But it can be a one variant - where you would to > put together expr with expr. And you can't do more from user space > now. But if you have a build in operator for (sqlidentifier, any) with > early processing - like "AS" in xml_attributies, we can do it. The > using of this operator can be limited only on function parameter > context. I'm sorry, I still don't follow. It creates an ordered pair, with one value being named "key" and the other one "val". Andwhen you use the ~> operator, the lhs is the key and the rhs if the value. > Postgres has a array of rows (Inside C or plperlu can be transofmed to > real hash simply). It just miss a user friendly notation for using it. I think Tom's right, frankly: An array of arrays will be the best solution for your interface. Sure someone could includemore than two items in each nested array, or fewer than 2, but if there are more you ignore them, and if there arefewer you treat them as NULLs. Best, David
2010/8/7 Tom Lane <tgl@sss.pgh.pa.us>:
> "David E. Wheeler" <david@kineticode.com> writes:
>> I think that some sort of variadic pairs would be useful for this. But since there is no core "ordered pair" data
type,I don't think you're going to get too far.
>
> It's not immediately clear to me what an ordered-pair type would get you
> that you don't get with 2-element arrays.
>
> A couple of quick experiments suggest that 2-D arrays might be the thing
> to use. They're easy to construct:
>
> regression=# select array[[1,2],[3,4]];
> array
> ---------------
> {{1,2},{3,4}}
> (1 row)
>
> and you can build them dynamically at need:
>
> regression=# select array[[1,2],[3,4]] || array[5,6];
> ?column?
> ---------------------
> {{1,2},{3,4},{5,6}}
> (1 row)
>
> This is not exactly without precedent, either: our built-in xpath()
> function appears to use precisely this approach for its namespace-list
> argument.
it's one variant, but isn't perfect
a) it expects so key and value are literals
b) it expects so all values has same types - what is usually what we
need, but not necessary
c) isn't too readable - I am sorry so I am repeating - it is same
reason, why people will prefer a VARIADIC function before function
with array - but I can accept, so this is my view of world
Regards
Pavel Stehule
>
> regards, tom lane
>
On Aug 6, 2010, at 9:59 PM, Tom Lane wrote:
> It's not immediately clear to me what an ordered-pair type would get you
> that you don't get with 2-element arrays.
Just syntactic sugar, really. And control over how many items you have (a bounded pair rather than an unlimited element
array).
> A couple of quick experiments suggest that 2-D arrays might be the thing
> to use. They're easy to construct:
>
> regression=# select array[[1,2],[3,4]];
> array
> ---------------
> {{1,2},{3,4}}
> (1 row)
>
> and you can build them dynamically at need:
>
> regression=# select array[[1,2],[3,4]] || array[5,6];
> ?column?
> ---------------------
> {{1,2},{3,4},{5,6}}
> (1 row)
>
> This is not exactly without precedent, either: our built-in xpath()
> function appears to use precisely this approach for its namespace-list
> argument.
Agreed.
Best,
David
On Aug 6, 2010, at 10:15 PM, Pavel Stehule wrote:
>> This is not exactly without precedent, either: our built-in xpath()
>> function appears to use precisely this approach for its namespace-list
>> argument.
>
> it's one variant, but isn't perfect
>
> a) it expects so key and value are literals
Huh? You can select into an array:
try=# create table foo(k text, v text);
CREATE TABLE
try=# insert into foo values ('foo', 'bar'), ('baz', 'yow');
INSERT 0 2
try=# select ARRAY[k,v] FROM foo; array
-----------{foo,bar}{baz,yow}
(2 rows)
try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
ERROR: could not find array type for datatype text[]
Okay, so nested arrays are harder.
> b) it expects so all values has same types - what is usually what we
> need, but not necessary
The XML interface converts them all to text anyway, no?
> c) isn't too readable - I am sorry so I am repeating - it is same
> reason, why people will prefer a VARIADIC function before function
> with array - but I can accept, so this is my view of world
I agree that it's not as sugary as pairs would be. But I admit to having no problem with
SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP.
Best,
David
2010/8/7 David E. Wheeler <david@kineticode.com>: > On Aug 6, 2010, at 9:48 PM, Pavel Stehule wrote: > >>> That's exactly what my solution does. The array solution doesn't. Whether it's appropriate to use a custom compositetype, however, is an open question. >> >> no it doesn't - in your design there are no different notation for key >> and for value. Next this design block a '->'. Because it's based on >> polymorphic operator. But it can be a one variant - where you would to >> put together expr with expr. And you can't do more from user space >> now. But if you have a build in operator for (sqlidentifier, any) with >> early processing - like "AS" in xml_attributies, we can do it. The >> using of this operator can be limited only on function parameter >> context. > > I'm sorry, I still don't follow. It creates an ordered pair, with one value being named "key" and the other one "val".And when you use the ~> operator, the lhs is the key and the rhs if the value. > >> Postgres has a array of rows (Inside C or plperlu can be transofmed to >> real hash simply). It just miss a user friendly notation for using it. > > I think Tom's right, frankly: An array of arrays will be the best solution for your interface. Sure someone could includemore than two items in each nested array, or fewer than 2, but if there are more you ignore them, and if there arefewer you treat them as NULLs. it can be a just plain text - it isn't about functionality, it is about readability, verbosity and stored procedures developer's comfort - and some consistency. Pavel > > Best, > > David > > >
2010/8/7 David E. Wheeler <david@kineticode.com>:
> On Aug 6, 2010, at 10:15 PM, Pavel Stehule wrote:
>
>>> This is not exactly without precedent, either: our built-in xpath()
>>> function appears to use precisely this approach for its namespace-list
>>> argument.
>>
>> it's one variant, but isn't perfect
>>
>> a) it expects so key and value are literals
>
> Huh? You can select into an array:
and pg doesn't handle 2D arrays well - can't to use ARRAY(subselect)
constructor for 2D arrays
>
> try=# create table foo(k text, v text);
> CREATE TABLE
> try=# insert into foo values ('foo', 'bar'), ('baz', 'yow');
> INSERT 0 2
> try=# select ARRAY[k,v] FROM foo;
> array
> -----------
> {foo,bar}
> {baz,yow}
> (2 rows)
>
> try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
> ERROR: could not find array type for datatype text[]
try SELECT ARRAY(SELECT row(k,v) FROM foo)
>
> Okay, so nested arrays are harder.
>
>> b) it expects so all values has same types - what is usually what we
>> need, but not necessary
>
> The XML interface converts them all to text anyway, no?
>
sure, but it isn't relevant here - the problem is buildin output
functions for datatypes. For example - true is different formated in
PostgresSQL and different formated in xml or JSON. Date values are
differently formated in JSON and XML. So if you would to correctly
format some date type value and if your interface is only text - then
you have to cast value back to binary and format it again. More - if
you have a information about original data type, you can use a corect
format. So if you use a only text parameters, then you lost a
significant information (when some parameter are not text). For
example, if I have only text interface for some hypothetical JSON API,
then I am not able to show a boolean value correctly - because it
doesn't use a quoting - and it is string and isn't number.
There is some other issue - PLpgSQL can't to work well with untyped
collections. But it isn't problem for C custom functions, and there
are not any reason why we can't to support polymorphic collections
(+/- because these collection cannot be accessed from PLpgSQL
directly).
>> c) isn't too readable - I am sorry so I am repeating - it is same
>> reason, why people will prefer a VARIADIC function before function
>> with array - but I can accept, so this is my view of world
>
> I agree that it's not as sugary as pairs would be. But I admit to having no problem with
>
> SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
>
> But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP.
>
Yes, when you are a author of code, you know what you are wrote. But
when you have do some review? Then an reviewer have to look on
definition of foo, and he has to verify, if you are use a parameters
well. For two params I don't see on first view what system you used -
[[key,key],[value,value]] or [[key,value],[key, value]]. More you have
to use a nested data structure - what is less readable then variadic
parameters. And - in pg - you are lost information about original data
types.
Regards
Pavel
> Best,
>
> David
>
>
>
On Aug 6, 2010, at 10:49 PM, Pavel Stehule wrote:
>> Huh? You can select into an array:
>
> and pg doesn't handle 2D arrays well - can't to use ARRAY(subselect)
> constructor for 2D arrays
Right.
>> try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
>> ERROR: could not find array type for datatype text[]
>
> try SELECT ARRAY(SELECT row(k,v) FROM foo)
Yeah, but those aren't nested arrays., They're…well, they're ordered pairs. ;-P
> sure, but it isn't relevant here - the problem is buildin output
> functions for datatypes. For example - true is different formated in
> PostgresSQL and different formated in xml or JSON. Date values are
> differently formated in JSON and XML. So if you would to correctly
> format some date type value and if your interface is only text - then
> you have to cast value back to binary and format it again. More - if
> you have a information about original data type, you can use a corect
> format. So if you use a only text parameters, then you lost a
> significant information (when some parameter are not text). For
> example, if I have only text interface for some hypothetical JSON API,
> then I am not able to show a boolean value correctly - because it
> doesn't use a quoting - and it is string and isn't number.
Point. FWIW, though, this is already an issue for non-SQL functions. PL/Perl, for example, gets all arguments cast to
text,AFAICT:
try=# create or replace function try(bool) returns text language plperl AS 'shift';
CREATE FUNCTION
Time: 121.403 ms
try=# select try(true);try
-----t
(1 row)
I wish this wasn't so.
> There is some other issue - PLpgSQL can't to work well with untyped
> collections. But it isn't problem for C custom functions, and there
> are not any reason why we can't to support polymorphic collections
> (+/- because these collection cannot be accessed from PLpgSQL
> directly).
I completely agree with you here. I'd love to be able to support RECORD arguments to non-C functions.
>> I agree that it's not as sugary as pairs would be. But I admit to having no problem with
>>
>> SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
>>
>> But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP..
>>
>
> Yes, when you are a author of code, you know what you are wrote. But
> when you have do some review? Then an reviewer have to look on
> definition of foo, and he has to verify, if you are use a parameters
> well. For two params I don't see on first view what system you used -
> [[key,key],[value,value]] or [[key,value],[key, value]]. More you have
> to use a nested data structure - what is less readable then variadic
> parameters. And - in pg - you are lost information about original data
> types.
Valid points. I agree that it would be nicer to use RECORDs:
SELECT foo( row('foo', 1), row('bar', true));
Certainly much clearer. But given that we've gone round and round on allowing non-C functions to use ROWs and gotten
nowhere,I don't know that we'll get any further now. But can you not create a C function that allows a signature of
VARIADICRECORD?
Best,
David
2010/8/7 David E. Wheeler <david@kineticode.com>:
> On Aug 6, 2010, at 10:49 PM, Pavel Stehule wrote:
>
>>> Huh? You can select into an array:
>>
>> and pg doesn't handle 2D arrays well - can't to use ARRAY(subselect)
>> constructor for 2D arrays
>
> Right.
>
>>> try=# select ARRAY(SELECT ARRAY[k,v] FROM foo);
>>> ERROR: could not find array type for datatype text[]
>>
>> try SELECT ARRAY(SELECT row(k,v) FROM foo)
>
> Yeah, but those aren't nested arrays., They're…well, they're ordered pairs. ;-P
>
>> sure, but it isn't relevant here - the problem is buildin output
>> functions for datatypes. For example - true is different formated in
>> PostgresSQL and different formated in xml or JSON. Date values are
>> differently formated in JSON and XML. So if you would to correctly
>> format some date type value and if your interface is only text - then
>> you have to cast value back to binary and format it again. More - if
>> you have a information about original data type, you can use a corect
>> format. So if you use a only text parameters, then you lost a
>> significant information (when some parameter are not text). For
>> example, if I have only text interface for some hypothetical JSON API,
>> then I am not able to show a boolean value correctly - because it
>> doesn't use a quoting - and it is string and isn't number.
>
> Point. FWIW, though, this is already an issue for non-SQL functions. PL/Perl, for example, gets all arguments cast to
text,AFAICT:
>
> try=# create or replace function try(bool) returns text language plperl AS 'shift';
> CREATE FUNCTION
> Time: 121.403 ms
> try=# select try(true);
> try
> -----
> t
> (1 row)
>
> I wish this wasn't so.
>
It must not be - it depends on PL handler implementation. PostgreSQL
call PL handler with binary values. I am thinking so new Python PL can
do it well.
>> There is some other issue - PLpgSQL can't to work well with untyped
>> collections. But it isn't problem for C custom functions, and there
>> are not any reason why we can't to support polymorphic collections
>> (+/- because these collection cannot be accessed from PLpgSQL
>> directly).
>
> I completely agree with you here. I'd love to be able to support RECORD arguments to non-C functions.
>
>>> I agree that it's not as sugary as pairs would be. But I admit to having no problem with
>>>
>>> SELECT foo(ARRAY[ ['foo', 'bar'], ['baz', 'yow']]);
>>>
>>> But maybe I'm biased, since there's a lot of that sort of syntax in pgTAP..
>>>
>>
>> Yes, when you are a author of code, you know what you are wrote. But
>> when you have do some review? Then an reviewer have to look on
>> definition of foo, and he has to verify, if you are use a parameters
>> well. For two params I don't see on first view what system you used -
>> [[key,key],[value,value]] or [[key,value],[key, value]]. More you have
>> to use a nested data structure - what is less readable then variadic
>> parameters. And - in pg - you are lost information about original data
>> types.
>
> Valid points. I agree that it would be nicer to use RECORDs:
>
> SELECT foo( row('foo', 1), row('bar', true));
I am not absolutly satisfied - but it's better, than arrays.
>
> Certainly much clearer. But given that we've gone round and round on allowing non-C functions to use ROWs and gotten
nowhere,I don't know that we'll get any further now. But can you not create a C function that allows a signature of
VARIADICRECORD?
you can do a variadic over ROW type. We have not a polymorphic arrays
- so isn't possible to write VARIADIC RECORD now. It could be a nice
if we are to define a own composite types with polymorphic fields.
Then you can do:
CREATE TYPE pair AS (key text, value "any");
CREATE FUNCTION foo(VARIADIC pair[])
other idea is leave arrays - and thinking about key, value collection
as new kind of data types. so maybe
CREATE FUNCTION foo(VARIADIC params COLECTION OF text WITH UNIQUE text KEY)
Regards
Pavel
>
> Best,
>
> David
>
>
>
>
>
>
>
>
>
>
>
On Aug 7, 2010, at 12:24 AM, Pavel Stehule wrote:
>> try=# create or replace function try(bool) returns text language plperl AS 'shift';
>> CREATE FUNCTION
>> Time: 121.403 ms
>> try=# select try(true);
>> try
>> -----
>> t
>> (1 row)
>>
>> I wish this wasn't so.
>>
>
> It must not be - it depends on PL handler implementation. PostgreSQL
> call PL handler with binary values. I am thinking so new Python PL can
> do it well.
I'm thinking an update to PL/Perl would be useful. Frankly, I'd most like to see proper array support. But that's
anothertopic.
>> Valid points. I agree that it would be nicer to use RECORDs:
>>
>> SELECT foo( row('foo', 1), row('bar', true));
>
> I am not absolutly satisfied - but it's better, than arrays.
>> Certainly much clearer. But given that we've gone round and round on allowing non-C functions to use ROWs and gotten
nowhere,I don't know that we'll get any further now. But can you not create a C function that allows a signature of
VARIADICRECORD?
>
> you can do a variadic over ROW type. We have not a polymorphic arrays
> - so isn't possible to write VARIADIC RECORD now.
Ah, right. I guess table types can't be cast to RECORD?
> It could be a nice
> if we are to define a own composite types with polymorphic fields.
> Then you can do:
>
> CREATE TYPE pair AS (key text, value "any");
> CREATE FUNCTION foo(VARIADIC pair[])
Yes.
> other idea is leave arrays - and thinking about key, value collection
> as new kind of data types. so maybe
>
> CREATE FUNCTION foo(VARIADIC params COLECTION OF text WITH UNIQUE text KEY)
COLLECTION?
David
2010/8/8 David E. Wheeler <david@kineticode.com>:
> On Aug 7, 2010, at 12:24 AM, Pavel Stehule wrote:
>
>>> try=# create or replace function try(bool) returns text language plperl AS 'shift';
>>> CREATE FUNCTION
>>> Time: 121.403 ms
>>> try=# select try(true);
>>> try
>>> -----
>>> t
>>> (1 row)
>>>
>>> I wish this wasn't so.
>>>
>>
>> It must not be - it depends on PL handler implementation. PostgreSQL
>> call PL handler with binary values. I am thinking so new Python PL can
>> do it well.
>
> I'm thinking an update to PL/Perl would be useful. Frankly, I'd most like to see proper array support. But that's
anothertopic.
>
>>> Valid points. I agree that it would be nicer to use RECORDs:
>>>
>>> SELECT foo( row('foo', 1), row('bar', true));
>>
>> I am not absolutly satisfied - but it's better, than arrays.
>
>>> Certainly much clearer. But given that we've gone round and round on allowing non-C functions to use ROWs and
gottennowhere, I don't know that we'll get any further now. But can you not create a C function that allows a signature
ofVARIADIC RECORD?
>>
>> you can do a variadic over ROW type. We have not a polymorphic arrays
>> - so isn't possible to write VARIADIC RECORD now.
>
> Ah, right. I guess table types can't be cast to RECORD?
>
>> It could be a nice
>> if we are to define a own composite types with polymorphic fields.
>> Then you can do:
>>
>> CREATE TYPE pair AS (key text, value "any");
>> CREATE FUNCTION foo(VARIADIC pair[])
>
> Yes.
>
>> other idea is leave arrays - and thinking about key, value collection
>> as new kind of data types. so maybe
>>
>> CREATE FUNCTION foo(VARIADIC params COLECTION OF text WITH UNIQUE text KEY)
>
> COLLECTION?
yes, sorry - simply - class where fields can be accessed via specified
index - unique or not unique.
>
> David
>
>
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:paramname="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
On Aug 7, 2010, at 11:05 PM, Pavel Stehule wrote: >> COLLECTION? > > yes, sorry - simply - class where fields can be accessed via specified > index - unique or not unique. Like in Oracle? From: http://download.oracle.com/docs/cd/B19306_01/appdev.102/b14261/collections.htm > A collection is an ordered group of elements, all of the same type. It is a general concept that encompasses lists, arrays,and other datatypes used in classic programming algorithms. Each element is addressed by a unique subscript. There are no keys. Best, David
2010/8/8 David E. Wheeler <david@kineticode.com>: > On Aug 7, 2010, at 11:05 PM, Pavel Stehule wrote: > >>> COLLECTION? >> >> yes, sorry - simply - class where fields can be accessed via specified >> index - unique or not unique. > > Like in Oracle? From: http://download.oracle.com/docs/cd/B19306_01/appdev.102/b14261/collections.htm yes, can be some like it. But I would to allow a non unique indexes > >> A collection is an ordered group of elements, all of the same type. It is a general concept that encompasses lists, arrays,and other datatypes used in classic programming algorithms. Each element is addressed by a unique subscript. > > There are no keys. ok - I didn't use a correct name - so "indexed set" is better. > > Best, > > David > >
Pavel Stehule wrote: > I didn't use a correct name - so "indexed set" is better. How would such a thing differ from a RAM-based local temporary table? -Kevin
On Aug 8, 2010, at 9:10 AM, Pavel Stehule wrote: >> There are no keys. > > ok - I didn't use a correct name - so "indexed set" is better. Hash? David
2010/8/8 Kevin Grittner <Kevin.Grittner@wicourts.gov>: > Pavel Stehule wrote: > >> I didn't use a correct name - so "indexed set" is better. > > How would such a thing differ from a RAM-based local temporary table? temporary tables are too heavy for this purposes. In SQL environment I expecting a transactional behave from tables. It isn't necessary. Next tables are strict structure. And it can be useless for storing a set of parameters. > > -Kevin > >
2010/8/8 David E. Wheeler <david@kineticode.com>: > On Aug 8, 2010, at 9:10 AM, Pavel Stehule wrote: > >>> There are no keys. >> >> ok - I didn't use a correct name - so "indexed set" is better. > > Hash? Just only hash isn't good model, because I sometimes we would prefer a ordered set Regards Pavel > > David > >
On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler <mike@mlfowler.com> wrote: > On 06/08/10 17:50, Pavel Stehule wrote: >> attached updated patch with regression test > > Bravely ignoring the quotation/varidic/<favourite_scheme_here> > conversations, Thank you! > I've taken a look at the patch as is. Excellent. > 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? Yes, I think that would be better. > 2) Pavel's regression test exposes a bug in libxslt. The stylesheet declares > 5 parameters, but uses 12. Simplifying, take the stylesheet: I'm not sure whether there's anything we can do about this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler <mike@mlfowler.com> wrote:
>> 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?
> Yes, I think that would be better.
I'm hesitant to consider pulling this into core when there's so little
consensus on how it ought to act. It'd be better to have a solid,
widely used contrib module *first*, rather than imagine that pulling it
into core is somehow a cure for its problems.
>> 2) Pavel's regression test exposes a bug in libxslt. The stylesheet declares
>> 5 parameters, but uses 12. Simplifying, take the stylesheet:
> I'm not sure whether there's anything we can do about this.
We should file a bug report with the libxslt authors, obviously.
regards, tom lane
On 09/08/10 04:07, Tom Lane wrote: > Robert Haas<robertmhaas@gmail.com> writes: >> On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler<mike@mlfowler.com> wrote: >>> 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? > >> Yes, I think that would be better. > > I'm hesitant to consider pulling this into core when there's so little > consensus on how it ought to act. It'd be better to have a solid, > widely used contrib module *first*, rather than imagine that pulling it > into core is somehow a cure for its problems. Perhaps the first step forward is to pull xslt_process out of xml2 and create a standalone xslt contrib module. Then at least it can lose the stigma of being in a deprecated module and perhaps make it more visible to users. > >>> 2) Pavel's regression test exposes a bug in libxslt. The stylesheet declares >>> 5 parameters, but uses 12. Simplifying, take the stylesheet: > >> I'm not sure whether there's anything we can do about this. > > We should file a bug report with the libxslt authors, obviously. Turns out the bug was filed in 2005 (see https://bugzilla.gnome.org/show_bug.cgi?id=307061). They are currently taking a fairly loose interpretation of the XSLT spec. However that was only one aspect of the concern. The other was that no errors were being reported back in psql when the libxslt is generating errors. Is this desirable? Regards, -- Mike Fowler Registered Linux user: 379787
Mike Fowler <mike@mlfowler.com> writes: > Turns out the bug was filed in 2005 (see > https://bugzilla.gnome.org/show_bug.cgi?id=307061). They are currently > taking a fairly loose interpretation of the XSLT spec. However that was > only one aspect of the concern. The other was that no errors were being > reported back in psql when the libxslt is generating errors. Is this > desirable? Uh, no; if we're failing to detect an error that the library does report, that's our bug (and another indication of the immaturity of this code :-()). regards, tom lane
On Mon, Aug 9, 2010 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Mike Fowler <mike@mlfowler.com> writes: >> Turns out the bug was filed in 2005 (see >> https://bugzilla.gnome.org/show_bug.cgi?id=307061). They are currently >> taking a fairly loose interpretation of the XSLT spec. However that was >> only one aspect of the concern. The other was that no errors were being >> reported back in psql when the libxslt is generating errors. Is this >> desirable? > > Uh, no; if we're failing to detect an error that the library does > report, that's our bug (and another indication of the immaturity > of this code :-()). Right. So, what about Mike's idea of extracting this into a new contrib module, perhaps contrib/xslt? That might also provide a good excuse to jettison any details of the existing interfaces that we happen to find unfortunate. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> writes:
> Right. So, what about Mike's idea of extracting this into a new
> contrib module, perhaps contrib/xslt? That might also provide a good
> excuse to jettison any details of the existing interfaces that we
> happen to find unfortunate.
Seems like mostly make-work to me --- we could just as easily fix the
code where it sits. But if you're excited about it, I won't stand in
the way.
regards, tom lane
Mike Fowler <mike@mlfowler.com> writes:
> 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've gone ahead and applied this patch, since the subsequent discussion
seemed to be getting *extremely* far afield from the expressed intent
of the patch, and nobody had pointed out a reason not to fix the
number-of-parameters limitation.
I think we have a few TODO items here:
* Invent ... and document ... an API that permits safe assembly of a
parameter list from non-constant (and perhaps untrustworthy) values.
* Fix xslt_process' failure to report (some?) errors detected by libxslt.
* Move the functionality to a less deprecated place.
None of these are within the scope of the current patch though.
regards, tom lane
Tom Lane wrote: > Mike Fowler <mike@mlfowler.com> writes: > > 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've gone ahead and applied this patch, since the subsequent discussion > seemed to be getting *extremely* far afield from the expressed intent > of the patch, and nobody had pointed out a reason not to fix the > number-of-parameters limitation. > > I think we have a few TODO items here: > > * Invent ... and document ... an API that permits safe assembly of a > parameter list from non-constant (and perhaps untrustworthy) values. > > * Fix xslt_process' failure to report (some?) errors detected by libxslt. > > * Move the functionality to a less deprecated place. > > None of these are within the scope of the current patch though. Should any of these be added to our TODO list under XML? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> I think we have a few TODO items here:
>>
>> * Invent ... and document ... an API that permits safe assembly of a
>> parameter list from non-constant (and perhaps untrustworthy) values.
>>
>> * Fix xslt_process' failure to report (some?) errors detected by libxslt.
>>
>> * Move the functionality to a less deprecated place.
>>
>> None of these are within the scope of the current patch though.
> Should any of these be added to our TODO list under XML?
Yes, all of them, since nothing's been done about any of 'em ...
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> I think we have a few TODO items here: > >> > >> * Invent ... and document ... an API that permits safe assembly of a > >> parameter list from non-constant (and perhaps untrustworthy) values. > >> > >> * Fix xslt_process' failure to report (some?) errors detected by libxslt. > >> > >> * Move the functionality to a less deprecated place. > >> > >> None of these are within the scope of the current patch though. > > > Should any of these be added to our TODO list under XML? > > Yes, all of them, since nothing's been done about any of 'em ... OK, TODO items added: Move XSLT from contrib/xml2 to a more reasonable location * http://archives.postgresql.org/pgsql-hackers/2010-08/msg00539.phpReport errors returned by the XSLT library * http://archives.postgresql.org/pgsql-hackers/2010-08/msg00562.phpImprove the XSLT parameter passing API * http://archives.postgresql.org/pgsql-hackers/2010-08/msg00416.php -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +