[patch_done] Strings like E'\\' are incorrectly tokenized

[EgonHugeist]

olehs,

I think you're a Postgre hero here which i'm not. So i think it might be better if you help us here.

I think we need to upgrade the function EncodeString of ZDbcPostgreUtils and need these additional parameters you proposed before.

What do you think?

Michael

[olehs]

Ok, I'll try.
The first step would be retrieving current standard_conforming_strings setting from the server instead of relying on server version.

I also introduced 2 convinience methods for TZPostgreSQLConnection: GetServerSetting & SetServerSetting.
I'm not familiar with other drivers, so maybe these method could be introduced in TZAbstractConnection/IZConnection?

[EgonHugeist]

olehs,

Ok, I'll try.
The first step would be retrieving current standard_conforming_strings setting from the server instead of relying on server version.

Thank you, each help is welcome! I checked your preview patch. Don't forget to close the ResultSet -> MemLeak

I also introduced 2 convinience methods for TZPostgreSQLConnection: GetServerSetting & SetServerSetting.
I'm not familiar with other drivers, so maybe these method could be introduced in TZAbstractConnection/IZConnection?

I don't think so. All database special things are easy to handle with the TZConnection.Properties. There i already introduces the Standart_Confirming_Strings value. ):

Hmm do you think we can upgrade the procedure with all your proposed parameters?

Michael

[olehs]

EgonHugeist,

I don't think so. All database special things are easy to handle with the TZConnection.Properties. There i already introduces the Standart_Confirming_Strings value. ):

You're right about TZConnection.Properties. But methods I proposed are made for getting and setting current run-time parameters.
I.e.

Code: Select all

 FStandardConformingStrings := Self.GetServerMajorVersion < 9;

is not a proper way of getting current state, because this parameter's default value can be overriden in server's config (and, actually, it's ON by default in 9+ and OFF in earlier versions).

It's better to retrieve it directly from server, and I introduced these methods for convinience

Code: Select all

      FStandardConformingStrings := UpperCase(GetServerSetting('standard_conforming_strings')) = 'ON';

Don't forget to close the ResultSet -> MemLeak

I thought it's closed in Destroyer, isn't it?

[EgonHugeist]

olehs,

All patches applyed. I do now run everything throgh the EncodeString procedure and added StatandartConfirmingStrings as Parameter.

I tested you proposal to revert the sequence like you propose. But the i got terrible results with the Test-Suites.

I attached you my test.properties file to show you how to setup the TestSuites. Copy that file into the \database folder, unpack this file, create a 'zeoslib' schema and run the tests. Go on with that good work and don't forget to ask for commit privileges.

Michael

[olehs]

EgonHugeist,

But the i got terrible results with the Test-Suites

It happened because of quotting string two times. GetEscapedString applies AnsiQoutedString to result of EncodeString, but EncodeString already added quotes around it.

[EgonHugeist]

olehs,

This i've chaged now. Rev 1611

But i can't get stable result if StandardConformingStrings=Off + escaping the backslashes. Now everyting will be handled from function EncodeString except a firs AnsiDequoteStr if the String is quoted...

Michael

[EgonHugeist]

olehs,

i added an empty test TestStandartConfirmingStrings to unit ZTestBugCompPostgreSql.pas.

It would be helpfull if you could all some tests to show me what's going wrong in your mind. What i understand is for example that i do only check the ' as quote char which is wrong in this case..

Michael

[olehs]

EgonHugeist,

I fixed QuoteState tokenizer, added support for nested comments in CommentState, added some tests.
I also had to remove backslash_quote processing, because it has no effect neither on tokenizing nor on escaping.

Now I have 2 questions:
First is about GetEscapeString. Strings starting with quote and ending with quote are DEquoted before processing. Don't you think this extra processing can confuse users. Is this behavior documented?

And second:
You told me to ask for commit priveleges, how do I do that?

[EgonHugeist]

olehs,

I fixed QuoteState tokenizer, added support for nested comments in CommentState, added some tests.
I also had to remove backslash_quote processing, because it has no effect neither on tokenizing nor on escaping.

Ok addad this because of your post... I know on splitting the tokens it was out of use(actually). But if slashes are valid quotes for Postgre and the are setting dependend then this should be supported too. So i prepared this too.

Now I have 2 questions:
First is about GetEscapeString. Strings starting with quote and ending with quote are DEquoted before processing. Don't you think this extra processing can confuse users. Is this behavior documented?

Nope, this is actually nowhere documented. And it is much more done for the Unicode stuff. This purpose becomes another one. I'll descibe this later on private email. So i'll send you a pm.

You told me to ask for commit priveleges, how do I do that?

i need your sourceforge.net username. then you need to checkout the repositories with a command line input like:
svn checkout --username=olehs https://svn.code.sf.net/p/zeoslib/code-0/trunk zeoslib-code-0
that's all.

Did upload 1623 today and saw since Postgre9 the export own escape functions like MySQL does it. Problem is we can use them only on the Plain-Layer. I'll write you tomorrow why. This is a little bit more complicatated because of the Unicode-Issue.

Michael

[olehs]

EgonHugeist,

I added a testcase for TestStandartConfirmingStrings (rev. 1625). It fails now besause of quotes I asked about.

[olehs]

EgonHugeist,

can you take a look at the following patch. What do you think about?

It gets standard_conforming_strings value from Properties, but if not specified - takes it depending on protocol version (ON - for PG9+).

This way user can specify SCS manually, or expect default behaviour for server version he uses.
We also can parse queries properly even before opening connection.
It also implements OnPropertiesChange event handler.

[EgonHugeist]

olehs,

I'll have a look to your fixes/proposals. Parsing the queries for special strings isn't the finest way(speed decrease). Here i think we'll find no end to keep everything like Zeoas want to have it. I propose to say: Hey user you're using a nice Component so do not play with default settings @runtime. There a some more things we've to check on parsing like the Encoding or datetime formats. So where to start and where to end here?

I'm dady now so don't wait for reply the next days..

Michael

[olehs]

Oh, this is great! Congratulations!!!

[EgonHugeist]

olehs,

Thank you !

Actually i didn't had the time to check your patch. Got a new Laptop: http://laptopia.de/ASUS-Notebooks/ASUS- ... ::327.html !! Wow lightnig fast!!

So the neext days i'll check it, ok?

Michael