Re: [HACKERS] Initial review of xslt with no limits patch

2011-02-18 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian br...@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.php 

Report errors returned by the XSLT library

* http://archives.postgresql.org/pgsql-hackers/2010-08/msg00562.php 

Improve the XSLT parameter passing API

* http://archives.postgresql.org/pgsql-hackers/2010-08/msg00416.php 

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] Initial review of xslt with no limits patch

2011-02-17 Thread Bruce Momjian
Tom Lane wrote:
 Mike Fowler m...@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  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] Initial review of xslt with no limits patch

2011-02-17 Thread Tom Lane
Bruce Momjian br...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-10 Thread Tom Lane
Mike Fowler m...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-09 Thread Mike Fowler

On 09/08/10 04:07, Tom Lane wrote:

Robert Haasrobertmh...@gmail.com  writes:

On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowlerm...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-09 Thread Tom Lane
Mike Fowler m...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-09 Thread Robert Haas
On Mon, Aug 9, 2010 at 10:14 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Mike Fowler m...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-09 Thread Tom Lane
Robert Haas robertmh...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Pavel Stehule
2010/8/8 David E. Wheeler da...@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 another topic.

 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 VARIADIC 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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Mike Fowler

On 06/08/10 17:50, Pavel Stehule wrote:


attached updated patch with regression test







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


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

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


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


xsl:stylesheet xmlns:xsl=http://www.w3.org/1999/XSL/Transform; 
version=1.0

   xsl:output method=xml omit-xml-declaration=yes indent=yes/
   xsl:strip-space elements=*/
   xsl:param name=n1/
   xsl:template match=*
 xsl:element name=samples
   xsl:element name=sample
 xsl:value-of select=$n1/
   /xsl:element
   xsl:element name=sample
 xsl:value-of select=$n2/
   /xsl:element
 /xsl:element
   /xsl:template
/xsl:stylesheet

and run the command:

~/temp$ xsltproc --stringparam n2 v2 Untitled2.xsl Untitled1.xml
samples
  sample/
  samplev2/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
  samplev1/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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread David E. Wheeler
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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Pavel Stehule
2010/8/8 David E. Wheeler da...@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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Kevin Grittner
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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread David E. Wheeler
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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Pavel Stehule
2010/8/8 Kevin Grittner kevin.gritt...@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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Pavel Stehule
2010/8/8 David E. Wheeler da...@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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Robert Haas
On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler m...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-08 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Sun, Aug 8, 2010 at 11:36 AM, Mike Fowler m...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-07 Thread David E. Wheeler
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 
VARIADIC RECORD?

Best,

David











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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-07 Thread Pavel Stehule
2010/8/7 David E. Wheeler da...@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 VARIADIC RECORD?

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












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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-07 Thread David E. Wheeler
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 another topic.

 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 VARIADIC 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?

David


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David Fetter
On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
 2010/8/6 Andrew Dunstan and...@dunslane.net:
  On 08/05/2010 06:56 PM, Mike Fowler wrote:
  SELECT
  xslt_process('employeenamecim/nameage30/agepay400/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 da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 David Fetter da...@fetter.org:
 On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:
 2010/8/6 Andrew Dunstan and...@dunslane.net:
  On 08/05/2010 06:56 PM, Mike Fowler wrote:
  SELECT
  xslt_process('employeenamecim/nameage30/agepay400/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 da...@fetter.org http://fetter.org/
 Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
 Skype: davidfetter      XMPP: david.fet...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Andrew Dunstan



On 08/06/2010 02:29 AM, Pavel Stehule wrote:

2010/8/6 David Fetterda...@fetter.org:

On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:

2010/8/6 Andrew Dunstanand...@dunslane.net:

On 08/05/2010 06:56 PM, Mike Fowler wrote:

SELECT
xslt_process('employeenamecim/nameage30/agepay400/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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Mike Fowler

On 06/08/10 15:08, Andrew Dunstan wrote:



On 08/06/2010 02:29 AM, Pavel Stehule wrote:

2010/8/6 David Fetterda...@fetter.org:

On Fri, Aug 06, 2010 at 05:57:37AM +0200, Pavel Stehule wrote:

2010/8/6 Andrew Dunstanand...@dunslane.net:

On 08/05/2010 06:56 PM, Mike Fowler wrote:

SELECT
xslt_process('employeenamecim/nameage30/agepay400/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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Mike Fowler m...@mlfowler.com writes:
 SELECT 
 xslt_process( ... , ... ,
  'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)

produces

 samples
samplev1/sample
samplev2/sample
samplev3/sample
samplev4/sample
samplev5/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+
   samplev1/sample+
   samplev2/sample+
   samplev3/sample+
   samplev4/sample+
   samplev5/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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
Hello

attached updated patch with regression test



2010/8/6 Tom Lane t...@sss.pgh.pa.us:
 Mike Fowler m...@mlfowler.com writes:
 SELECT
 xslt_process( ... , ... ,
              'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text)

 produces

 samples
    samplev1/sample
    samplev2/sample
    samplev3/sample
    samplev4/sample
    samplev5/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            +
   samplev1/sample+
   samplev2/sample+
   samplev3/sample+
   samplev4/sample+
   samplev5/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

*** ./contrib/xml2/expected/xml2.out.orig	2010-02-28 22:31:57.0 +0100
--- ./contrib/xml2/expected/xml2.out	2010-08-06 18:46:41.0 +0200
***
*** 145,147 
--- 145,215 
  Value/attribute/attributes');
  create index idx_xpath on t1 ( xpath_string
  ('/attributes/attribu...@name=attr_1]/text()', xml_data::text));
+ SELECT xslt_process('employeenamecim/nameage30/agepay400/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 name=sample
+ xsl:value-of select=$n6/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n7/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n8/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n9/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n10/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n11/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n12/
+   /xsl:element
+ /xsl:element
+   /xsl:template
+ /xsl:stylesheet$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5,n6=v6,n7=v7,n8=v8,n9=v9,n10=v10,n11=v11,n12=v12'::text);
+   xslt_process  
+ 
+  samples +
+samplev1/sample +
+samplev2/sample +
+samplev3/sample +
+samplev4/sample +
+samplev5/sample +
+samplev6/sample +
+samplev7/sample +
+samplev8/sample +
+samplev9/sample +
+samplev10/sample+
+samplev11/sample+
+samplev12/sample+
+  /samples+
+  
+ (1 row)
+ 
*** ./contrib/xml2/sql/xml2.sql.orig	2010-08-06 18:30:00.0 +0200
--- ./contrib/xml2/sql/xml2.sql	2010-08-06 18:30:57.0 +0200
***
*** 80,82 
--- 80,132 
  
  create index idx_xpath on t1 ( xpath_string
  ('/attributes/attribu...@name=attr_1]/text()', xml_data::text));
+ 
+ SELECT xslt_process('employeenamecim/nameage30/agepay400/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 name=sample
+ xsl:value-of select=$n6/
+   /xsl:element
+   xsl:element name=sample
+ xsl:value-of select=$n7/
+   

Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Andrew Dunstan



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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Andrew Dunstan and...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 Tom Lane t...@sss.pgh.pa.us:
 Andrew Dunstan and...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 2010/8/6 Tom Lane t...@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/values
one variadic array element - parse it the old way
otherwise - 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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 Robert Haas robertmh...@gmail.com:
 On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule pavel.steh...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Robert Haas robertmh...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Pavel Stehule pavel.steh...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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 ordered pair` as the signature.

I don't suppose anyone has implemented a data type like this…

Best,

David


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Peter Eisentraut
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 may pass multiple name/value pairs up to a
   maximum of 32. If the value being passed is a string, you can use
   --stringparam instead, 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 is a string rather than a node identifier.  Note:
   The string must be UTF-8 encoded.



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Peter Eisentraut
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. :-)


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 David E. Wheeler da...@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 `variadic ordered 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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 Peter Eisentraut pete...@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. :-)

:)

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
 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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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 name for =).

Best,

David



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Robert Haas
On Fri, Aug 6, 2010 at 1:46 PM, Pavel Stehule pavel.steh...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/6 David E. Wheeler da...@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 new name 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




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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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 new name 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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
Peter Eisentraut pete...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
David E. Wheeler da...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David Fetter
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 da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/7 Tom Lane t...@sss.pgh.pa.us:
 David E. Wheeler da...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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 better than 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.

Best,

David


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/7 David E. Wheeler da...@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 better than 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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Tom Lane
David E. Wheeler da...@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

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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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. 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 include more than two items in each nested 
array, or fewer than 2, but if there are more you ignore them, and if there are 
fewer you treat them as NULLs.

Best,

David



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/7 Tom Lane t...@sss.pgh.pa.us:
 David E. Wheeler da...@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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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


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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread David E. Wheeler
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



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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/7 David E. Wheeler da...@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 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. 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 include more than two items in each 
 nested array, or fewer than 2, but if there are more you ignore them, and if 
 there are fewer 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




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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-06 Thread Pavel Stehule
2010/8/7 David E. Wheeler da...@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




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


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-05 Thread Mike Fowler

Hi Pavel,

On 02/08/10 09:21, Pavel Stehule wrote:

Hello

2010/8/2 Mike Fowlerm...@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('employeenamecim/nameage30/agepay400/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
  samplev1/sample
  samplev2/sample
  samplev3/sample
  samplev4/sample
  samplev5/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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-05 Thread Pavel Stehule
2010/8/6 Mike Fowler m...@mlfowler.com:
 Hi Pavel,

 On 02/08/10 09:21, Pavel Stehule wrote:

 Hello

 2010/8/2 Mike Fowlerm...@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('employeenamecim/nameage30/agepay400/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
  samplev1/sample
  samplev2/sample
  samplev3/sample
  samplev4/sample
  samplev5/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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-05 Thread Andrew Dunstan



On 08/05/2010 06:56 PM, Mike Fowler wrote:


SELECT 
xslt_process('employeenamecim/nameage30/agepay400/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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-05 Thread Pavel Stehule
2010/8/6 Andrew Dunstan and...@dunslane.net:


 On 08/05/2010 06:56 PM, Mike Fowler wrote:

 SELECT
 xslt_process('employeenamecim/nameage30/agepay400/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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Initial review of xslt with no limits patch

2010-08-02 Thread Pavel Stehule
Hello

2010/8/2 Mike Fowler m...@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.0 +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
*** ./contrib/xml2/xslt_proc.c.orig	2010-07-06 21:18:55.0 +0200
--- ./contrib/xml2/xslt_proc.c	2010-08-02 09:31:32.410911283 +0200
***
*** 41,49 
  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 */
  
  
--- 41,50 
  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 */
  
  
***
*** 57,63 
  	text	   *doct = PG_GETARG_TEXT_P(0);
  	text	   *ssheet = PG_GETARG_TEXT_P(1);
  	text	   *paramstr;
! 	const char *params[MAXPARAMS + 1];	/* +1 for the terminator */
  	xsltStylesheetPtr stylesheet = NULL;
  	xmlDocPtr	doctree;
  	xmlDocPtr	restree;
--- 58,64 
  	text	   *doct = PG_GETARG_TEXT_P(0);
  	text	   *ssheet = PG_GETARG_TEXT_P(1);
  	text	   *paramstr;
! 	const char **params;
  	xsltStylesheetPtr stylesheet = NULL;
  	xmlDocPtr	doctree;
  	xmlDocPtr	restree;
***
*** 69,79 
  	if (fcinfo-nargs == 3)
  	{
  		paramstr = PG_GETARG_TEXT_P(2);
! 		parse_params(params, paramstr);
  	}
  	else
  		/* No parameters */
  		params[0] = NULL;
  
  	/* Setup parser */
  	pgxml_parser_init();
--- 70,83 
  	if (fcinfo-nargs == 3)
  	{
  		paramstr = PG_GETARG_TEXT_P(2);
! 		params = parse_params(paramstr);
  	}
  	else
+ 	{
  		/* No parameters */
+ 		params = palloc(sizeof(char *));
  		params[0] = NULL;
+ 	}
  
  	/* Setup parser */
  	pgxml_parser_init();
***
*** 139,160 
  
  #ifdef USE_LIBXSLT
  
! static void
! parse_params(const char **params, text *paramstr)
  {
  	char	   *pos;
  	char	   *pstr;
- 	int			i;
  	char	   *nvsep = =;
  	char	   *itsep = ,;
! 
  	pstr = text_to_cstring(paramstr);
  
  	pos = pstr;
! 
! 	for (i = 0; i  MAXPARAMS; i++)
  	{
! 		params[i] = pos;
  		pos = strstr(pos, nvsep);
  		if (pos != NULL)
  		{
--- 143,175 
  
  #ifdef USE_LIBXSLT
  
! const char **
! parse_params(text *paramstr)
  {
  	char	   *pos;
  	char	   *pstr;
  	char	   *nvsep = =;
  	char	   *itsep = ,;
! 	const char **params;
! 	int	nparams;
! 	int	mparams;		/* max params */
! 	
  	pstr = text_to_cstring(paramstr);
+ 	
+ 	mparams = INIT_PARAMS;
+ 	params = (const char **) palloc(INIT_PARAMS * sizeof(char *) + 1);
  
  	pos = pstr;
! 	nparams = 0;
! 	while (*pos != '\0')
  	{
! 		if (nparams = mparams)
! 		{
! 			/* extend params params */
! 			mparams += EXTEND_PARAMS;
! 			params = (const char **) repalloc(params, mparams * sizeof(char *) + 1);
! 		}
! 		params[nparams++] = pos;
  		pos = strstr(pos, nvsep);
  		

[HACKERS] Initial review of xslt with no limits patch

2010-08-01 Thread Mike Fowler

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.0 +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

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