My answers inline.
Daniel John Debrunner wrote:
So if I understand you correctly the SQL statement of the above format should give an error.-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Shreyas Kaushik wrote:
Yes it does.
Daniel John Debrunner wrote:
Does the fix correctly implement duplicate name checking as detailed in this mail?
http://mail-archives.apache.org/eyebrowse/[EMAIL PROTECTED]&msgNo=1569
Hmmm, I don't see how the change in FromList (extracted below) will correctly handle
select ... FROM A.T1 as C, B.T2 as C
It seems that the leftTable variable will be set to <A.C> and rightTable
set to <B.C> which will not compare as equal. Am I missing something? If
the patch had tests I could see if this case was handled.
That being the case it does take care of this case, giving the error as shown below:
ij> select * from app."S1.T1" as C, s1.t1 as C;
ERROR 42X09: The table or alias name 'C' is used more than once in the FROM list.
I can change the naming for these, what could be some possible variable names that are suitable.The code in FromList is not that clear to me, could be helped by comments, and it concerns me that a TableName object is created out of a schema name and a correlation name when such a construct has no valid meaning. I see that you are hampered by the fact that there is no way to tell if a correlation name has been used. In msgNo 1569 I laid out that it would be best if there were two getExposed methods, but now I think a single String getExposedName() will do, as long as it returns null if no correlation name was used. That might have a ripple effect though the code, but the best way to fix a bug is sometimes a minor rework, rather than just band-aids on existing bad code. I think the fact that exposed name handling has three bugs shows a re-work is needed. In any re-work I would start out with explicitly stating how exposed names are handled for a table in the from list and for a column reference, and then how the two uses relate and compare.
Other comments on the patch are:
- - for the code extract below as a minor nit I would avoid using terms
like 'leftTable' or 'rightTable' unless left and right actual have
meaning, which they don't here. One table in a list is being compared
against the others in the list.
- - The other set of changes related to getAllResultColumns() again have
the possible issue of creating a TableName out of a schema name and
correlation name. I assume these are to fix derby-12, again comments
would be useful.
I can add the comments here.
Also any comments you have on if this fixes Derby-18 and Derby-12 would
be helpful.
It takes care of Derby-12. But Derby-18 is slightly more than what this patch tries to address. (Derby -18 is related to column binding when qualified with a schema name)
This patch does not fully cover Derby-18.
Dan.
- - if (fromTable.getExposedName().equals - - (((FromTable) elementAt(index)).getExposedName()) ) + leftTable = makeTableName( fromTable.getTableName().getSchemaName(), + fromTable.getExposedName()); + + if(((FromTable) elementAt(index)).getTableName() == null) { + rightTable = makeTableName(null, ((FromTable) elementAt(index)).getExposedName()); + } + + else { + rightTable = makeTableName(((FromTable) elementAt(index)).getTableName().getSchemaName(), + ((FromTable) elementAt(index)).getExposedName()); + } + if(leftTable.equals(rightTable)) { throw StandardException.newException(SQLState.LANG_FROM_LIST_DUPLICATE_TABLE_NAME, fromTable.getExposedName()); }
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.5 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFB44TBIv0S4qsbfuQRAjgCAKC7xhogSSJgQS3YeGVLxiM3ePdBmgCgqGd2 jRQZ5MXv5Z6nti9/r5iwQDo= =FFKt -----END PGP SIGNATURE-----
