Thread: 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.
Attachment
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.
Attachment
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.
Attachment
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.
Attachment
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.
Attachment
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.
Attachment
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