Bug in TZDefaultIdentifierConvertor.GetIdentifierCase?

[duzenko]

Code: Select all

{** written by FrOsT}
function TZDefaultIdentifierConvertor.GetIdentifierCase(
  const Value: String; TestKeyWords: Boolean): TZIdentifierCase;
...
begin
...
    { Checks for reserved keywords. }
    if Metadata.GetDatabaseInfo.StoresUpperCaseIdentifiers and (Result <> icUpper)
    then S := UpperCase(Value)
    else s := LowerCase(Value);

I got Metadata.GetDatabaseInfo.StoresUpperCaseIdentifiers = true, Result = icUpper
The branch above causes S to be lowercase and it always fails the compare with the upper-case keywords
Please advise

[marsupilami]

Hello duzenko,

as my crystal ball that helps me in knowing what configuration you have is in service currently please answer some questions:
- Which version of Zeos do you use? Do you use the latest ZIP or do you get your Zeos from SVN?
- Which RDBMS do you use? And which Version of that RDBMS?
- Which OS do you use and which version of that OS?
- Which Version of Delphi or Lazarus/FPC do you use?

Best regards,

Jan

[duzenko]

- Which version of Zeos do you use? Do you use the latest ZIP or do you get your Zeos from SVN?
Stable 7.2.4
- Which RDBMS do you use? And which Version of that RDBMS?
Firebird 2.5
- Which OS do you use and which version of that OS?
Windows 10
- Which Version of Delphi or Lazarus/FPC do you use?
2007

Nothing of the above seems to be related to the question on hand
Surely if S is "time" and sql keyword is "TIME" they won't match.

[marsupilami]

Could you please check the current SVN version of Zeos 7.2? We did some changes there on cases like this. I am just not sure if they apply to Firebird or not. If the error persists, could you provide example code and a database script that we can use for testing?

[duzenko]

Thanks, the svn 7.2 seems to have fixed that
Now I have a totally different failure with the boolean -> char conversion

Test table structure

Code: Select all

CREATE TABLE TEST_CHAR_BOOL
(
  SOME_STRING Varchar(10),
  SOME_BOOL Char(1)
);

I use a TZTable to append a record

Code: Select all

procedure TForm3.Button1Click(Sender: TObject);
begin
  ZTable1.Append;
  ZTable1['some_string'] := IntToStr(Random(999));
  ZTable1['some_bool'] := Random(2) = 1;
  ZTable1.Post;
end;

It crashes with

Project Project1.exe raised exception class EZSQLException with message 'SQL Error: Dynamic SQL ErrorSQL error code = -303arithmetic exception, numeric overflow, or string truncationstring right truncation. Error Code: -303. Incompatible column/host variable data type The SQL: INSERT INTO TEST_CHAR_BOOL (SOME_STRING,SOME_BOOL) VALUES (?,?); '.

Stack trace

Code: Select all

:76a21812 KERNELBASE.RaiseException + 0x62
ZDbcInterbase6Utils.CheckInterbase6Error(???,???,$25E91A0,lcExecute,'INSERT INTO TEST_CHAR_BOOL (SOME_STRING,SOME_BOOL) VALUES (?,?)')
ZDbcInterbase6Statement.TZInterbase6PreparedStatement.ExecuteInternal
ZDbcInterbase6Statement.TZInterbase6PreparedStatement.ExecuteUpdatePrepared
ZDbcGenericResolver.TZGenericCachedResolver.PostUpdates(TZAbstractCachedResultSet($26468A0) as IZCachedResultSet,???,???,$263FEE0)
ZDbcCachedResultSet.TZAbstractCachedResultSet.PostRowUpdates(???,???)
ZDbcCachedResultSet.TZAbstractCachedResultSet.PostUpdates
ZDbcCachedResultSet.TZAbstractCachedResultSet.InsertRow
ZAbstractDataset.TZAbstractDataset.InternalAddRecord(???,False)
ZAbstractDataset.TZAbstractDataset.InternalPost
DB.TDataSet.CheckOperation((ZAbstractDataset.TZAbstractDataset.InternalPost,$2B6ED90),(nil,nil))
DB.TDataSet.Post
Unit1.TForm3.Button1Click(???)
Controls.TControl.Click
StdCtrls.TButton.Click
StdCtrls.TButton.CNCommand(???)
Controls.TControl.WndProc(???)
Controls.TWinControl.WndProc((48401, 3268, 6294724, 0, 3268, 0, 3268, 96, 0, 0))
StdCtrls.TButtonControl.WndProc(???)
Controls.TControl.Perform(???,???,6294724)
Controls.DoControlMsg(???,(no value))
Controls.TWinControl.WMCommand((273, 3268, 0, 6294724, 0))
Forms.TCustomForm.WMCommand(???)
Controls.TControl.WndProc(???)
Controls.TWinControl.WndProc((273, 3268, 6294724, 0, 3268, 0, 3268, 96, 0, 0))
Forms.TCustomForm.WndProc(???)
Controls.TWinControl.MainWndProc(???)

Both 7.2 (stable and svn) crash like this while the 7.1 I'm on "just works" (C) - I suppose something changed in how zeoslib converts delphi boolean to sql char under the hood

The expected behavior (and observed on 7.1) is that it would save either 'F' or 'T' to the char(1) column while I suspect 7.2 tries to push the entire "False" or "True" into it?

Please advise

[duzenko]

FWIW the DB is UTF-8 and TZAbstractRODataset.InternalInitFieldDefs seems to multiply the actual size of the field by 4 to calculate the field size
Then the string representation of True or False gets clamped to 4 chars and that ends up sent to FB server

[duzenko]

Found the svn commit that broke that

Revision: 3756
Author: egonhugeist
Date: Friday, 22 January 2016 21:17:08
Message:
finalize the FIELDSIZE story!

3755 was the last to work

[marsupilami]

The expected behavior (and observed on 7.1) is that it would save either 'F' or 'T' to the char(1) column while I suspect 7.2 tries to push the entire "False" or "True" into it?

Zeos 7.1 - or more correctly Delphi - tried the same. But the Zeos Firebird driver silently truncated the string values True and False to T and F in Zeos 7.1 whereas Zeos 7.2 (correctly) forwards the whole string to Firebird which then tells you that the String you want to store (True / False) doesn't fit into the field where you want to store it - the "string right truncation" in the firebird message.

My advice: Use the field as a char field. Something like this might make sense:

Code: Select all

function MyBoolToStr(const x: Boolean): String;
begin
  case x of
    True: Result := 'T';
    False: Result := 'F';
  end;
end;

procedure TForm3.Button1Click(Sender: TObject);
begin
  ZTable1.Append;
  ZTable1['some_string'] := IntToStr(Random(999));
  ZTable1['some_bool'] := MyBoolToStr(Random(2) = 1);
  ZTable1.Post;
end;

Best regards,

Jan

[Fr0sT]

Boolean won't automagically transform to Char. Previous behavior was incorrect. If you want to operate Booleans, FB3+ is your option.

[duzenko]

Sorry, do you understand that this is a backward incompatible, breaking change?
I just can't update literally hundreds of my customers' Firebird servers to 3.0.
Was it announced anywhere that starting from 7.2 Zeoslib no longer supports bool's as firebird 2.5 char's?
Was it discussed on this forum before the decision was made?
Did you read my comment above that the string value is truncated to 4 chars while the field itself is 1 char?
No offense intended, but It just sounds like you're trying to dress a bug as a feature.

You seem to be missing the point that Zeoslib is truncating the string to 4 char's while it should truncate to 1. It's not about truncate or not - it's about 4 or 1.

Zeos 7.2 (correctly) forwards the whole string to Firebird

Except it does not. It just truncates to more char's than 7.1 (and in no way that is 'correct')
You still need to truncate to real field length in character, but do that on a WideString rather than AnsiString

[duzenko]

I am talking about TZAbstractRODataset.InternalInitFieldDefs and how it forces x4 field sizes on utf-8 DB even though ftString is not supposed to do anything with Unicode. The right way is to return ftWideString from the driver and use the actual field size.
At least, you need to make the x4 stuff optional.

[marsupilami]

Sorry, do you understand that this is a backward incompatible, breaking change?

Yes, we do.

I just can't update literally hundreds of my customers' Firebird servers to 3.0.

In this case you have bigger problems than a behaviour change in Zeos because at some point you will have to do exactly that - update client databases to Firebird 3.0 or 4.0 of 5.0 or ... I suggest you start planning this.

Was it announced anywhere that starting from 7.2 Zeoslib no longer supports bool's as firebird 2.5 char's?

There is no such support as using bools as Firbird Char(1) fields. You relied on behaviour that worked but was not intentionally working. At least from a Zeos point of view.

Was it discussed on this forum before the decision was made?

No - it wasn't. Do we need your approval before we make changes to the Zeos code? Do you want to step up and support or do Zeos development? If you don't want to do any of that then why do you demand that we discuss the development of Zeos with you before we do changes?

Did you read my comment above that the string value is truncated to 4 chars while the field itself is 1 char?

We did the decision that we support users pushing UTF8 directly to the database using TStringField in ANSI Delphis. In this use case the user is responsible for making sure that not more than the allowed amount of characters gets pushed into the TString Field. Even if Zeos would do some kind of check here, it wouldn't be a truncation but raising an exception that there are too many characters stored in the field. To me all this sounds like you have set TZConnection.ClientCodepage to UTF8? Why would one do that on an ANSI Delphi?

No offense intended, but It just sounds like you're trying to dress a bug as a feature.

Too bad - the way you write here, I already did take offense.

You seem to be missing the point that Zeoslib is truncating the string to 4 char's while it should truncate to 1. It's not about truncate or not - it's about 4 or 1.

Zeos 7.2 (correctly) forwards the whole string to Firebird

Except it does not. It just truncates to more char's than 7.1 (and in no way that is 'correct')
You still need to truncate to real field length in character, but do that on a WideString rather than AnsiString

Again: Zeos does no intentional truncation. Delphi is doing the truncation in this example because it happens on this line of your example code:

Code: Select all

ZTable1['some_bool'] := Random(2) = 1;

During this assignment Delphi decides to truncate the string value of "False" to Fals or to F - depending on TStringField.Size. Zeos (correctly) just sends the value that gets stored in the StringField to the database that in turn correctly raises an exception.

Soo - to bring this to an end:

• To me it seems that your connection character set is UTF8 while you use an ANSI Delphi. Why? This simply doesn't make sense. You might want to consider using the correct ANSI character set for your database connection. ZConnection.ClientCodepage='WIN1252' for example.
• Another option might be to use ClientCodepage=UTF8 and ControlsCodePage=cCP_UTF16. On the SVN version of Zeos 7.2 this will make Zeos generate TWideStingField instead of TStringField with the correct number of characters. I am not 100% sure if the release version Zeos 7.2.4 will behave in the same way. Watch out for type casts like TStringField(DataSet.FieldByName('somefield')) in your code in that case because they might be incorrect afterwards and cause access violations then.

Best regards,

Jan

[Fr0sT]

So you rely on the following chain:
- having Boolean
- when assigned to string field, implicitly converted with BoolToStr
- string value copied to string field AND silently truncated to that field size
- value of 1st char in Delphi's boolean string representation is then written to DB
As for me, this bunch of implicit behaviors is apparently a bad practice not suitable for production. Moreover I would raise an exception like FB's 'string truncation' on field assignment but Delphi seems to be a forgiver.

I'd advice to not use char fields for booleans at all. Char fields bring many problems with encodings. Proper way of imitating booleans is a smallint domain with check that value is [0,1]. This way you won't have hidden string conversions and stuff. Or it might allow any value in the range implementing "everything <>0 is true" logic.

For your case might be an option to use:
- your own TField descendant with any conversions you wish
- or TField.OnSet/GetText handlers.

[duzenko]

So you rely on the following chain:
- having Boolean
- when assigned to string field, implicitly converted with BoolToStr
- string value copied to string field AND silently truncated to that field size
- value of 1st char in Delphi's boolean string representation is then written to DB
As for me, this bunch of implicit behaviors is apparently a bad practice not suitable for production. Moreover I would raise an exception like FB's 'string truncation' on field assignment but Delphi seems to be a forgiver.

I'd advice to not use char fields for booleans at all. Char fields bring many problems with encodings. Proper way of imitating booleans is a smallint domain with check that value is [0,1]. This way you won't have hidden string conversions and stuff. Or it might allow any value in the range implementing "everything <>0 is true" logic.

For your case might be an option to use:
- your own TField descendant with any conversions you wish
- or TField.OnSet/GetText handlers.

No, I'm pretty sure the bad practice is to misrepresent a char(1) field as a char(4)
The char(1) is one of the officially recommended alternative for a boolean field in Furebird 2.5 http://www.firebirdfaq.org/faq12/
It worked very well with Zeoslib for many years until someone decided to 'fix' it
I'm not going to comment on your personal opinion - you're entitled to those but please don't serve it as final truth
Maybe you should take a break and then come back and re-read my posts above - this has NOTHING to do with booleans. The BUG is that 7.2 returning INVALID field length for ALL char fields in a UTF-8 database.

[duzenko]

Well, my problem is the above mentioned breaking change was not only mentioned NOWHERE in the docs or the forum but also you defend it as a 'fix' while to me it looks you just found out about it. Am I wrong and it's a common knowledge? Did anyone anywhere on this forum or elsewhere bring it up yet? Do you think that being upset about this is unreasonable?

We did the decision that we support users pushing UTF8 directly to the database using TStringField in ANSI Delphis.

ANSI Delphis have TWideStringField exactly for that purpose. That's where you were wrong at the start and everything else is consequences.

In this use case the user is responsible for making sure that not more than the allowed amount of characters gets pushed into the TString Field.

And the field length checks in TStringField flush down the drain. It's only something Delphi programmers relied on for decades now - no big deal.

There is no such support as using bools as Firbird Char(1) fields. You relied on behaviour that worked but was not intentionally working. At least from a Zeos point of view.

No support for meaningful TField.Size values as well?

Even if Zeos would do some kind of check here, it wouldn't be a truncation but raising an exception that there are too many characters stored in the field.

You don't have to do any checks at all, just return the actual field length. And surely not to try to convert WideString to Utf-8 or vice versa.

No - it wasn't. Do we need your approval before we make changes to the Zeos code? Do you want to step up and support or do Zeos development? If you don't want to do any of that then why do you demand that we discuss the development of Zeos with you before we do changes?

I want to read on the reasons why char(1) has Size=1 in 7.1 and 4 in 7.2. If there were any. The breaking changes need to be explained - am I asking too much?
I have done small contributions in the past and I don't see what it has to do with anything.

Too bad - the way you write here, I already did take offense.

Why, was it your personal decision to quadruple the advertised field size?

You still need to truncate to real field length in character, but do that on a WideString rather than AnsiString

TStringField already does that (did until you broke it in 7.2). No way I am running around the source code manually truncating each and every field I'm sending to the database. And surely it has nothing to do with WideStrings unless you confuse AnsiStrings with utf8's.

Again: Zeos does no intentional truncation. Delphi is doing the truncation in this example because it happens on this line of your example code:

Code: Select all

ZTable1['some_bool'] := Random(2) = 1;

During this assignment Delphi decides to truncate the string value of "False" to Fals or to F - depending on TStringField.Size.

God forbid you started truncating stuff in Zeos on top on what you already messed up with field Size.

Zeos (correctly) just sends the value that gets stored in the StringField to the database that in turn correctly raises an exception.

Oh yeah? And where does the TStringField.Size come from?

Soo - to bring this to an end:

• To me it seems that your connection character set is UTF8 while you use an ANSI Delphi. Why? This simply doesn't make sense. You might want to consider using the correct ANSI character set for your database connection. ZConnection.ClientCodepage='WIN1252' for example.

Because the DB is used by other applications but mine and some of them require the DB to be UTF-8.

[*]Another option might be to use ClientCodepage=UTF8 and ControlsCodePage=cCP_UTF16. On the SVN version of Zeos 7.2 this will make Zeos generate TWideStingField instead of TStringField with the correct number of characters. I am not 100% sure if the release version Zeos 7.2.4 will behave in the same way. Watch out for type casts like TStringField(DataSet.FieldByName('somefield')) in your code in that case because they might be incorrect afterwards and cause access violations then.[/list]

That changes exactly nothing. It's still Size=4 for char(1) with the inevitable runtime errors.

Soo - to bring this to an end:

• Consider one of the following
• Ban my forum account cause I'm not going to agree and call this bug an 'improvement'
• Agree to optionally return the ACTUAL field size
• Agree to optionally use TWideStringField's on unicode databases instead of TStringField