Fix for MySQL GetIndexInfo listing all indexes as unique.

[MJFShark]

In unit ZDbcMySqlMetadata, function TZMySQLDatabaseMetadata.UncachedGetIndexInfo

replace the line:
Result.UpdateString(IndexInfoColNonUniqueIndex, LowerCase(BoolStrs[GetInt(ColumnIndexes[2]) = 0]));

with
Result.UpdateBoolean(IndexInfoColNonUniqueIndex, (GetInt(ColumnIndexes[2]) = 1));

-Mark

[aehimself]

@ Mark,

How did you get this error? I'm using Zeos with MySQL on a daily basis and never seen it returning everything as unique.
Do you maybe have a test case?

Cheers!

[MJFShark]

That's odd. I've tested it quite a bit. What do you get if you put this code in that function (at that same line)? Note that the dataset's type for IndexInfoColNonUniqueIndex is a boolean and the existing code tries to set it as a string (and I think the logic of it is incorrect regardless.)

Code: Select all

        Result.UpdateString(IndexInfoColNonUniqueIndex, 'true');
        Assert(Result.GetBoolean(IndexInfoColNonUniqueIndex) = True, 'failed: should be true');

I've tracked it down to a bug in function
StrToBoolEx(Buf, PEnd: PAnsiChar; const CheckInt: Boolean = True;
const IgnoreTrailingSaces: Boolean = True): Boolean; overload;

Code: Select all

    Ord('t'): if Pend-Buf = 1 then
                Result := True
              else Result := (Pend-Buf = 4) and (Ord((Buf+1)^) or $20 = Ord('r')) and
                (Ord((Buf+1)^) or $20 = Ord('u')) and (Ord((Buf+1)^) or $20 = Ord('e'));

Note that above the (Buf+1)s should be +1, +2, +3, etc.

I think both fixes should be implemented.

-Mark

[MJFShark]

Note that if we just fix the bug in StrToBoolEx without fixing the logic of the GetIndexes call then the NONUNIQUE value will be reversed, so it's important to do both.

-Mark

[MJFShark]

The bug I mentioned above in StrToBoolEx() exists in two of the overloads and affects both the 'yes' check and the 'true' check I mentioned above (due to the buffer position not being incremented properly beyond +1)

So the two overloads affected are:

function StrToBoolEx(Buf, PEnd: PAnsiChar; const CheckInt: Boolean = True;
const IgnoreTrailingSaces: Boolean = True): Boolean; overload;

and

function StrToBoolEx(Buf, PEnd: PWideChar; const CheckInt: Boolean = True;
const IgnoreTrailingSaces: Boolean = True): Boolean; overload;

In both cases the fix is to change the (Buf+1)^ code to the correct offsets for the later characters.

-Mark

[aehimself]

That's odd. I've tested it quite a bit. What do you get if you put this code in that function (at that same line)? Note that the dataset's type for IndexInfoColNonUniqueIndex is a boolean and the existing code tries to set it as a string (and I think the logic of it is incorrect regardless.)

Don't get me wrong - I trust you that you debugged it and of course it's possible that Zeos has bugs :)
My question was mainly directed towards the "real world" example. I used Zeos from MySQL 5.2 (now upgraded to 8.something on a steady upgrade path) and Zeos never seemed to get my indexes wrong.

[MJFShark]

The real world test is to call GetIndexInfo on a MySQL table that contains both a primary key and also a non-unique index. You should see the NON-UNIQUE column for the non-unique index = false (which is incorrect.) The only guess I can think of for your Meta.GetIndexInfo calls to work correctly is that you're using a db character set such that one of the "working" overloads of StrToBoolEx() is being used, but even if that's true the logic is backwards. It appears to report primary keys as "NON-UNIQUE" = True (it should be false.) I believe the changes above are correct and will work in all cases.

I have the same table in Oracle, MySQL, PostgreSQL and Firebird. All are using UTF-8 character sets. The results for Oracle, Pg, and Firebird are:

TABLE_NAME,INDEX_NAME,NON_UNIQUE
species,species_pkey,False
species,fk_genusid_index,True

This info is correct. The primary key index is unique (of course) and so the NON_UNIQUE column is false. The second index is a non-unique index for a foreign key and shows NON_UNIQUE as true (which is correct.)

In my tests MySQL shows both as false, and if I fix the StrToBoolEx bugs it shows them reversed. I'm not sure how yours could seem to work correctly even if my guess on the character set making a difference is correct. If you do see correct results for the above, please let me know as it means that I'm somehow corrupt or not up to date (which I did check btw, perhaps even correctly )

-Mark

[marsupilami]

The bug I mentioned above in StrToBoolEx() exists in two of the overloads and affects both the 'yes' check and the 'true' check I mentioned above (due to the buffer position not being incremented properly beyond +1)

So the two overloads affected are:

function StrToBoolEx(Buf, PEnd: PAnsiChar; const CheckInt: Boolean = True;
const IgnoreTrailingSaces: Boolean = True): Boolean; overload;

and

function StrToBoolEx(Buf, PEnd: PWideChar; const CheckInt: Boolean = True;
const IgnoreTrailingSaces: Boolean = True): Boolean; overload;

In both cases the fix is to change the (Buf+1)^ code to the correct offsets for the later characters.

-Mark

Hello Mark,

I fixed both overloads for Zeos 8.0. Thank you for pointing this out. Could you please take a look? I will also add your fix for TZMySQLDatabaseMetadata.UncachedGetIndexInfo later on to see if any of the tests start to fail.

Best regards,

Jan

[MJFShark]

Thanks Jan. The fixes for StrToBoolEx look good. Sorry about doing this as a user patch instead of a pr though github, but I changed machines recently and I'm still working on getting things setup.

Now that that fix is in the reversed NON_UNIQUE flags for MySQL's GetIndexInfo should become more obvious.

-Mark

[marsupilami]

I also included your fix for TZMySQLDatabaseMetadata.UncachedGetIndexInfo. Thanks for fixing it

[aehimself]

The real world test is to call GetIndexInfo on a MySQL table that contains both a primary key and also a non-unique index. You should see the NON-UNIQUE column for the non-unique index = false (which is incorrect.)

Strange, even in my own home-cooked application I have plenty of tables like this and I never ran into problems. I do have to admit though I did not check the metadata that often. When I get home from my well-deserved vacation I'll make sure to check this out.

I am using Queries though and Zeos's internal logic normally relies on metadata... maybe the small amount of test data I'm generating never fulfilled a condition to throw an error until now.

The only reason I'm this interested is that MySQL is my main RDBMS with Zeos for 6+ years now and feel kind of superficial for not even seeing it :cry:

Anyway, thank you for the fix :)

[Fr0sT]

Seems like good thing to add to test suite

[MJFShark]

@aehimself: Enjoy your vacation! This only caused non-unique indexes to be reported incorrectly, so I don't think it's any kind of major issue really, but good to get it fixed so thanks all. I really only use the getindexinfo call for informational display, not really for any major program logic (unlike GetPrimaryKey and GetImportedKeys.)

-Mark

[marsupilami]

Seems like good thing to add to test suite

It is and I am willing to accept patches. But unfortunately I don't have the time to create a test right now.
However - I am willing to support anybody who wants to write a test. If necessary, I could also come up with something like a small tutorial if that helps.

[Fr0sT]

I took a glance on StrToBoolEx and was surprised to find plenty of unit tests. How that serious bug with "+1" could even pass the tests?