Page 1 of 1

Duplicate field name: FIELD_1

Posted: 04.06.2020, 09:30
by aehimself
I have a table with a field, named FIELD_1. I join that table on itself:

SQLQuery.SQL.Text := 'SELECT * FROM Table JOIN Table x ON x.ID = Table.Joined';
SQLQuery.Open;

I am greeted with the error:

EDatabaseError was raised while running background operation with the message: SQLQuery: Duplicate field name 'FIELD_1'
Oracle 12.1.0, client version: 19.6.0, database component version: 7.3.0-3ed53109 (7.2 more fixes to the proxy driver. Some enable it to compile on Delphi XE 10.2 again.)

As far as I see Data.DB is creating the fields from FieldDefList (Data.DB.pas, line 12925):

Code: Select all

        Field := FieldDef.CreateField(Self, nil, FieldDefList.Strings[I]);
I guess Zeos is NOT appending _1 to the end of the field name when populating this, as it thinks it's already there and is trying to create two fields with the same name.

Re: Duplicate field name: FIELD_1

Posted: 04.06.2020, 14:40
by aehimself
Okay, so the issue is close, but is not what I thought.

I have a table with 4 columns:
ID
FIELD
FIELD_1
PARENTCOLUMNID

I join this table on itself:

SELECT * FROM TABLE
JOIN TABLE t ON t.ID = TABLE.PARENTCOLUMNID

The error is in TZAbstractResultSetMetadata.GetColumnLabel, when it reaches the second FIELD named field. It cycles through the already existing columns, increasing "N" each time the name is the same. As only one FIELD named field exists, N is 1, and GetColumnLabel will append _1 to the field name. Problem? FIELD_1 already exists.

I have a proposed fix, I just don't know how you guys would like the result...

Change the cycle in ZDbcResultSetMetadata.pas line 426 from:

Code: Select all

      for J := 0 to I - 1 do
        if TZColumnInfo(ColumnsInfo[J]).ColumnLabel = ColumnName then
          Inc(N);
To:

Code: Select all

      for J := 0 to I - 1 do
        if (TZColumnInfo(ColumnsInfo[J]).ColumnLabel = ColumnName) Or
           (TZColumnInfo(ColumnsInfo[J]).ColumnLabel = ColumnName + '_' + ZFastCode.IntToStr(N)) then
          Inc(N);
With this, the above query instead of failing shows the following columns:
ID, FIELD, FIELD_1, PARENTCOLUMNID, ID_1, FIELD_2, FIELD_3, PARENTCOLUMNID_1

I don't like it that much, but I don't have a better idea. If you are fine with this, I can issue a pull request on GitHub.

Re: Duplicate field name: FIELD_1

Posted: 05.06.2020, 08:52
by marsupilami
Hello aehimself,

to me it seems like Eginhugeist already applied your patches in revision 6609 / 1d1ab16.

Best regards,

Jan

Re: Duplicate field name: FIELD_1

Posted: 05.06.2020, 11:17
by aehimself
Nice. A little bit more elegant solution, too I have to admit.

Unfortunately I can not test it as current GIT revision does not compile (ZIBEventAlerter is looking for CheckInterbase6Error and FParent.FFBConnection.HandleError, which does not exist).

In the mean time I'm attempting to gather some information on an other issue I just faced - a custom function declared in the database which is working fine from PL/SQL and a .NET application is throwing ORA-06502 when called from Zeos:
bug.png
But this is going to be a different topic when I have a test case :)

Re: Duplicate field name: FIELD_1

Posted: 05.06.2020, 11:49
by aehimself
aehimself wrote: 05.06.2020, 11:17In the mean time I'm attempting to gather some information on an other issue I just faced - a custom function declared in the database which is working fine from PL/SQL and a .NET application is throwing ORA-06502 when called from Zeos:
But this is going to be a different topic when I have a test case :)
Nop, it won't be. Found the issue and it has nothing to do with Zeos ^^ yayy

Re: Duplicate field name: FIELD_1

Posted: 05.06.2020, 14:00
by aehimself
marsupilami wrote: 05.06.2020, 08:52Hello aehimself,

to me it seems like Eginhugeist already applied your patches in revision 6609 / 1d1ab16.

Best regards,

Jan
7.3.0-1d1ab16e works for the test case but unfortunately it does not work for real life. In reality my database table contains 5 versions of the same field:

FIELD
FIELD_1
FIELD_2
FIELD_3
FIELD_4

@Eginhugeis't checkin fails at the burned-in 2 cycles from this perspective. Enumerates first table's fields fine. While enumerating the second (self-joined) table, it looks like this (talking about the flow of TZAbstractResultSetMetadata.GetColumnLabel):
- ColumnName is set to "FIELD"
- First FOR cycle, pass 1 / 2. Second FOR cycle goes through the existing fields, and finds a field named "FIELD" already. It increases N with one.
- Second FOR exits, ColumnName is being changed to "FIELD_1". First FOR exits.
- First FOR cycle, pass 2 / 2. Second FOR cycle goes through the existing fields, and finds a field named "FIELD_1" already. It increases N with one.
- Second FOR exits, ColumnName is being changed to "FIELD_2". First FOR exists.
- First FOR has no more cycles. Final determined field name is FIELD_2.
At a later stage, CreateField is called with this name, which will throw an exception, as FIELD_2 exists too.

This can be changed to an "endless" loop, until field name is not found:

Code: Select all

//      for B := False to True do begin //we need two loops to have a unique columnlabel
      Repeat
        b := False;
        //see test TestDuplicateColumnNames or
        //https://zeoslib.sourceforge.io/viewtopic.php?f=50&t=120692
        for J := 0 to I - 1 do
          if TZColumnInfo(ColumnsInfo[J]).ColumnLabel = ColumnName then
            Begin
              Inc(N);
              b := True;
            End;
        if N > 0 then
          ColumnName := OrgLabel + '_' + ZFastCode.IntToStr(N);
      Until Not b;
Or we can use my first advised solution, checking if the original OR the next possible field name exists. I don't know which would have less resource footprint.

Edit: I was looking at how PL/SQL is handling this. It is not adding _1 to the end of the field name, simply two fields named "FIELD" shows up in the resultset. I started to wonder. What if we use random names (DB Guids / field positions, whatever) for Field.FieldName and set Field.DisplayName to the original field name? Grids would show the field name as it is in the database and we still could create the dataset. Question is, how much extra work it would mean to fix .FieldByName and all the other methods.
Just an idea, though.

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 11:30
by marsupilami
aehimself wrote: 05.06.2020, 14:00 Edit: I was looking at how PL/SQL is handling this. It is not adding _1 to the end of the field name, simply two fields named "FIELD" shows up in the resultset. I started to wonder. What if we use random names (DB Guids / field positions, whatever) for Field.FieldName and set Field.DisplayName to the original field name? Grids would show the field name as it is in the database and we still could create the dataset. Question is, how much extra work it would mean to fix .FieldByName and all the other methods.
Just an idea, though.
EgonHugeist wanted to apply your first proposition.

Regarding FieldName versus DisplayName: We don't have something that matches the DisplayName property on the DBC layer, so can't provide that information for TZQuery to use.
Even if we had the possibililty, I would not be too keen in using it because it hides the field name that can be used to access the second field in a case like this:

select t1.a, t2.a from table1 join table2

There would be no easy way for a developer or a user to know what the internally used field name for t2.a is. A use case for this would be a program that allows users to design their own reports by using SQL.

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 11:49
by aehimself
marsupilami wrote: 08.06.2020, 11:30EgonHugeist wanted to apply your first proposition.
Do you mean the "endless" loop method? I can change it, test it and issue a pull request with the fix; if all is fine with this solution.

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 12:19
by marsupilami
Please do that :)

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 13:43
by aehimself
marsupilami wrote: 08.06.2020, 12:19 Please do that :)
"Endless" loop method implemented, tested and sent as pull request. With the introduction of OrgLabel the results are even more... strange, but at least closer to reality:

TABLE.FIELD
TABLE.FIELD_1
TABLE.FIELD_2
TABLE.FIELD_3
TABLE.FIELD_4
JOINEDTABLE.FIELD_5
JOINEDTABLE.FIELD_1_1
JOINEDTABLE.FIELD_2_1
JOINEDTABLE.FIELD_3_1
JOINEDTABLE.FIELD_4_1

I guess we will live with this "inconvenience". Let this be a lesson: Don't design your databases as poorly as us :D

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 17:11
by marsupilami
The patch is applied :)
aehimself wrote: 08.06.2020, 13:43 I guess we will live with this "inconvenience". Let this be a lesson: Don't design your databases as poorly as us :D
Honestly I don't think, this is a bad database design. In our databases all tables have an ID field. For me this is a bad query design -> explicitly name your fields if you do something like this. Also only ask for the fields you need. :)

Best regards,

Jan

Re: Duplicate field name: FIELD_1

Posted: 08.06.2020, 17:39
by aehimself
marsupilami wrote: 08.06.2020, 17:11For me this is a bad query design -> explicitly name your fields if you do something like this. Also only ask for the fields you need. :)
In the application queries are written "normally". Only include fields that are being accessed, renaming them as necessary. When I am debugging some invalid returned data, in my database application I usually write "SELECT * FROM" because I'm lazy to type.
This is how I met this issue :)