Обсуждение: dollar quoting

Hello!

I've added dollar quoting support to the 8.2dev-503 version query parser.

Where should I send the patch?

--
Jure Koren, unix developer
Hehe d.o.o.

Jure Koren wrote:
> Hello!
>
> I've added dollar quoting support to the 8.2dev-503 version query parser.
>
> Where should I send the patch?

I have just recently sent a patch for dollar quoting to this list and I am
waiting for feedback now. :-)

Michael

Jure,

Please send a context diff to this list, and perhaps you may want to
read this

http://archives.postgresql.org/pgsql-jdbc/2006-10/msg00000.php

Dave
On 3-Oct-06, at 11:54 AM, Jure Koren wrote:

> Hello!
>
> I've added dollar quoting support to the 8.2dev-503 version query
> parser.
>
> Where should I send the patch?
>
> --
> Jure Koren, unix developer
> Hehe d.o.o.

On Tuesday 03 October 2006 17:54, Jure Koren wrote:
> Hello!
> I've added dollar quoting support to the 8.2dev-503 version query parser.

Well, it's really not that big, so here it is.

Though it works for me and I've tried to read the dollar quoting definition in
the postgresql docs several times, I can't really be sure if it's correct :)

If anyone bothers enough with it to find a problem, you can let me know.

Regards,

--
Jure Koren, unix developer
Hehe d.o.o.

On Tuesday 03 October 2006 19:00, Jure Koren wrote:
> Well, it's really not that big, so here it is.

And, naturally, I attached the one that is broken.

Here it is, again.

--
Jure Koren, unix developer
Hehe d.o.o.

Jure,


On 3-Oct-06, at 1:00 PM, Jure Koren wrote:

> On Tuesday 03 October 2006 17:54, Jure Koren wrote:
>> Hello!
>> I've added dollar quoting support to the 8.2dev-503 version query
>> parser.
>
> Well, it's really not that big, so here it is.
>
> Though it works for me and I've tried to read the dollar quoting
> definition in
> the postgresql docs several times, I can't really be sure if it's
> correct :)
>
> If anyone bothers enough with it to find a problem, you can let me
> know.

Isn't StringBuilder a jdk 1.5 only function ?
>
> Regards,
>
> --
> Jure Koren, unix developer
> Hehe d.o.o.
> <postgresql-jdbc-dollarquote.diff>

Dave Cramer <pg@fastcrypt.com> writes:
>> On Tuesday 03 October 2006 17:54, Jure Koren wrote:
>>> I've added dollar quoting support to the 8.2dev-503 version query
>>> parser.

> Isn't StringBuilder a jdk 1.5 only function ?

Also, this patch is not being nearly cautious enough about what is a
valid dollar-quote start sequence.  The other patch that was proposed
looked more reasonable on that score.

            regards, tom lane

On Tuesday 03 October 2006 20:14, Tom Lane wrote:
> Also, this patch is not being nearly cautious enough about what is a
> valid dollar-quote start sequence.  The other patch that was proposed
> looked more reasonable on that score.

Yes.

This is the version that I think addresses most of the concerns posted.

Thank you for your feedback.

--
Jure Koren, unix developer
Hehe d.o.o.

On Tuesday 03 October 2006 21:38, Jure Koren wrote:
> This is the version that I think addresses most of the concerns posted.

Except for the empty ($$) tag, which this, hopefully last version, does as
well.

Sorry, i haven't slept for a while.

--
Jure Koren, unix developer
Hehe d.o.o.

Jure Koren <jure@hehe.si> writes:
>> This is the version that I think addresses most of the concerns posted.
> Except for the empty ($$) tag, which this, hopefully last version, does as
> well.

No ... a locale-dependent character class test is exactly the wrong
thing.  See the other discussion.

            regards, tom lane

Jure Koren wrote:
> On Tuesday 03 October 2006 21:38, Jure Koren wrote:
>> This is the version that I think addresses most of the concerns posted.
>
> Except for the empty ($$) tag, which this, hopefully last version, does as
> well.

Some more comments:
- I don't think the regular expression is quite there yet, look at the code
in the other patch -- the "c > 127" part

- Having said that, I don't believe in compiling a regular expression for
each dollar-quoted string and each $ character that is not part of a dollar
quote

- There must be something wrong with this line at the end of a dollar-quote:
   i = nextpos + tag.length() + 2;
My other patch has this here:
   i += dollarQuoteTag.length() - 1;

"nextpos" in the first case should be the same as "i" in the second case.

Something like ..$$String$$; SELECT ... will break here, I guess. My patch
included JUnit tests to detect such off-by-one-or-more errors. You could
use them.

I am now wondering which version has the better performance... especially I
would like to know if switching between scanning the char[] and the
original query string in this patch has any negative impact. If not, it
could be applied quite easily to the other one. Perhaps you could evaluate
both versions for performance?

Best Regards,
Michael Paesold

On Tuesday 03 October 2006 21:54, Tom Lane wrote:
> No ... a locale-dependent character class test is exactly the wrong
> thing.  See the other discussion.

But it isn't locale dependent. All unicode letter class character match,
regardless of locale (which is what the postgres docs say as well).

Yes, probably not the best thing to do, but my reasoning was that dollar
quoting is going to be used by humans writing functions. Using dollar
escaping in non-ddl stuff is, i think, extremely unlikely, as normal escaping
functions will normally take care of those cases.

Michael Paesold <mpaesold@gmx.at> wrote:
> - Having said that, I don't believe in compiling a regular expression for
> each dollar-quoted string and each $ character that is not part of a dollar
> quote

Yes, and I've noticed there's a missing test for leading whitespace, which
would make this fail very fast if it in fact wasn't a dollar quote. I'm
incapable of producing any more code today, but I'll be certainly improving
it later this week.

I'm not actually familiar with Java, so verbose criticism is most welcome,
off-list too. And i'm subscribed to the list, so you don't have to make
copies :)

Thank you for all the feedback and good night.

--
Jure Koren, unix developer
Hehe d.o.o.

Jure Koren <jure@hehe.si> writes:
> On Tuesday 03 October 2006 21:54, Tom Lane wrote:
>> No ... a locale-dependent character class test is exactly the wrong
>> thing.  See the other discussion.

> But it isn't locale dependent. All unicode letter class character match,
> regardless of locale (which is what the postgres docs say as well).

The point is that it's the wrong thing because it doesn't match what the
backend does: all characters > 127 are allowed in dollar tags, no matter
whether Java thinks they are letters or not.

            regards, tom lane