Re: Initial review of xslt with no limits patch

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Mike Fowler <mike(at)mlfowler(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Initial review of xslt with no limits patch
Date: 2010-08-02 08:21:13
Message-ID: AANLkTi=cpLP=0dPbwPri3SfqPU6er-ZitJFZR2iZP1H5@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

2010/8/2 Mike Fowler <mike(at)mlfowler(dot)com>:
> Hi Pavel,
>
> Currently your patch isn't applying to head, from the looks of things a
> function signature has changed. Can you update your patch please?
>

yes - see attachment

> Also, having had a read through the patch itself I note that there are no
> tests and no changes to documentation. Shouldn't the documentation advertise
> that the there are no limits on the numbers of parameters? A couple of tests
> will also help me to test your patch,
>

the limit of 10 parameters was not documented. With this patch, there
are not limit - or limit comes from libxml2.

Test are a problem - for me - I don't understand to xslt too much - so
I am not able to write xslt template with more than 10 params.

I looked there and I the params are not tested now - so it is
necessary to write a new set of regress tests. But I am not able to do
it :(.

> Below is the results of running patch:
>
> patch -p0 < ../nolimits.diff
> patching file ./contrib/xml2/xslt_proc.c
> Hunk #1 FAILED at 42.
> Hunk #2 succeeded at 57 (offset -2 lines).
> Hunk #3 succeeded at 69 (offset -2 lines).
> Hunk #4 succeeded at 142 (offset -4 lines).
> Hunk #5 succeeded at 179 (offset -4 lines).
> Hunk #6 succeeded at 192 with fuzz 1 (offset -4 lines).
> 1 out of 6 hunks FAILED -- saving rejects to file
> ./contrib/xml2/xslt_proc.c.rej
>
> The rejects were:
>
>
> *** ./contrib/xml2/xslt_proc.c.orig     2010-03-03 20:10:22.000000000 +0100
> --- ./contrib/xml2/xslt_proc.c  2010-05-03 15:07:17.010918303 +0200
> ***************
> *** 42,50 ****
>  extern void pgxml_parser_init(void);
>
>  /* local defs */
> ! static void parse_params(const char **params, text *paramstr);
>
> ! #define MAXPARAMS 20                  /* must be even, see parse_params()
> */
>
>  #endif /* USE_LIBXSLT */
>
> --- 42,51 ----
>  extern void pgxml_parser_init(void);
>
>  /* local defs */
> ! const char **parse_params(text *paramstr);
>
> ! #define INIT_PARAMS 20                        /* must be even, see
> parse_params() */
> ! #define EXTEND_PARAMS 20              /* must be even, see parse_params()
> */
>
>  #endif /* USE_LIBXSLT */
>
>
> Regards,
> --
> Mike Fowler
> Registered Linux user: 379787
>

Regards

Pavel Stehule

Attachment Content-Type Size
xslt.patch text/x-patch 3.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2010-08-02 08:45:40 Re: Synchronous replication
Previous Message Markus Wanner 2010-08-02 07:36:24 Re: english parser in text search: support for multiple words in the same position